Re: pg_createsubscriber - more logging to say if there are no pubs to drop - Mailing list pgsql-hackers

On Tue, Oct 14, 2025 at 5:26 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Wed, Oct 15, 2025 at 11:11 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Tue, Oct 14, 2025 at 4:29 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > On Wed, Oct 15, 2025 at 4:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Wed, Oct 8, 2025 at 6:34 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > > > >
> > > > > Hi hackers,
> > > > >
> > > > > While reviewing pg_createsubscriber in another thread, I found some of
> > > > > the current logging to be confusing. Specifically, there is one part
> > > > > that drops all existing publications. Sometimes it might look like
> > > > > this:
> > > > >
> > > > > ----------
> > > > > pg_createsubscriber: dropping all existing publications in database "db2"
> > > > > pg_createsubscriber: dropping publication "pub_exists1" in database "db2"
> > > > > pg_createsubscriber: dropping publication "pub_exists2" in database "db2"
> > > > > pg_createsubscriber: dropping publication "pub_exists3" in database "db2"
> > > > > ----------
> > > > >
> > > > > ~~~
> > > > >
> > > > > OTOH, if there is nothing found to be dropped, then the logging just says:
> > > > >
> > > > > ----------
> > > > > pg_createsubscriber: dropping all existing publications in database "db2"
> > > > > ----------
> > > > >
> > > > > That's the scenario that I found ambiguous. You can't be sure from the
> > > > > logs what happened:
> > > > > - Were there publications found, and were they dropped silently?
> > > > > - Did it not find anything to drop?
> > > > >
> > > > > ~~~
> > > > >
> > > > > Here is a small patch to remove that doubt. Now, if there is nothing
> > > > > found, the logging would look like:
> > > > >
> > > > > ----------
> > > > > pg_createsubscriber: dropping all existing publications in database "db2"
> > > > > pg_createsubscriber: no publications found
> > > > > ----------
> > > > >
> > > > > Thoughts?
> > > >
> > > > Thank you for the patch!
> > > >
> > > > It sounds like a reasonable improvement. I'll push the patch, barring
> > > > any objections.
> > > >
> > >
> > > Hm.. I'm wondering now if this patch was correct :-(
> > >
> > > I had encountered this problem and made this patch while reviewing
> > > another thread [1], but that other patch is still in development flux
> > > and so the logging was changing, and may have been broken at the time
> > > I wrote this one.
> > >
> > > In hindsight, I am not sure if this "no publication found" log is
> > > working as intended for the master code.
> > >
> > > ~~~
> > >
> > > For example, during a '--dry-run' the create/drop publication in
> > > pg_createsubscriber are NOP but they still do the appropriate logging.
> > >
> > > So, it is true that PQntuples(res) can be 0, because for dry run
> > > create_publication is NOP.
> > > But the dry run will still show logging for the create and for the drop.
> > >
> > > I hacked one of the test cases:
> > >
> > > # PS HACK
> > > command_ok(
> > >     [
> > >         'pg_createsubscriber',
> > >         '--verbose',
> > >         '--dry-run',
> > >         '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default,
> > >         '--pgdata' => $node_s->data_dir,
> > >         '--publisher-server' => $node_p->connstr($db1),
> > >         '--socketdir' => $node_s->host,
> > >         '--subscriber-port' => $node_s->port,
> > >         '--publication' => 'pub1',
> > >         '--publication' => 'pub2',
> > >         '--subscription' => 'sub1',
> > >         '--subscription' => 'sub2',
> > >         '--database' => $db1,
> > >         '--database' => $db2,
> > >         '--clean' => 'publications',
> > >     ],
> > >     'PS HACK');
> > >
> > > This gives a logging result like:
> > >
> > > pg_createsubscriber: dropping all existing publications in database "xxx"
> > > 2025-10-15 10:18:53.583 AEDT client backend[31402]
> > > 040_pg_createsubscriber.pl LOG:  statement: SELECT pubname FROM
> > > pg_catalog.pg_publication;
> > > pg_createsubscriber: no publications found
> > > pg_createsubscriber: dropping publication "pub2" in database "xxx"
> > >
> > > ~~~
> > >
> > > First saying "no publications found", and then logging a drop anyway
> > > may seem contrary to the user.
> > >
> > > That was not the intended outcome for this patch.
> >
> > Hmm. So the subscriber should have at least one publication in
> > non-dry-run cases, and in dry-run cases it has no subscription but
> > shows the "dropping publication" log, is that right? If the newly
> > added log could rather confuse users, we should revert the change.
> >
>
> IIUC the created publication (on publisher) ends up on the subscriber
> because of the physical replication that occurred before the tool
> switches over to use logical replication.
>
> So the created publication has ended up on subscriber, and now the
> --clean will remove it.
>
> But for dry-run that create_publication is NOP so it never *actually*
> propagated to the subscriber, so the fetch finds nothing to delete (it
> says "no publication found") but there will still be a
> drop_publication logged in dry-run mode.
>
> The bottom line is I think this patch can be reverted because it
> didn't succeed to make things clearer, and may from some point-of-view
> even make things less clear.

Agreed - I've pushed the changes now. While the change seemed
straightforward, I misunderstood the --dry-run cases. I apologize for
that. Thank you for pointing it out.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com