Re: pg_createsubscriber - more logging to say if there are no pubs to drop

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.

Sorry for the churn.

======
Kind Regards,
Peter Smith
Fujitsu Australia