Horton

Thread

Commits

  1. Revert "pg_createsubscriber: Add log message when no publications exist to drop."

  2. pg_createsubscriber: Add log message when no publications exist to drop.

  1. pg_createsubscriber - more logging to say if there are no pubs to drop

    Peter Smith <smithpb2250@gmail.com> — 2025-10-09T01:33:25Z

    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?

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

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

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-10-14T17:41:38Z

    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.

Regards,

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

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

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-10-14T18:49:06Z

    On Tue, Oct 14, 2025 at 10:41 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.

Done.

Regards,

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

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

    Peter Smith <smithpb2250@gmail.com> — 2025-10-14T21:18:15Z

    On Wed, Oct 15, 2025 at 5:49 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Oct 14, 2025 at 10:41 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Wed, Oct 8, 2025 at 6:34 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
...
> > >
> > > 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.
>
> Done.
>

Thanks for pushing!

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

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

    Peter Smith <smithpb2250@gmail.com> — 2025-10-14T23:29:32Z

    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.

======
[1] https://www.postgresql.org/message-id/flat/CAHv8Rj%2BsxWutv10WiDEAPZnygaCbuY2RqiLMj2aRMH-H3iZwyA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

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

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-10-15T00:10:41Z

    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.


Regards,

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

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

    Peter Smith <smithpb2250@gmail.com> — 2025-10-15T00:26:14Z

    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

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

    Masahiko Sawada <sawada.mshk@gmail.com> — 2025-10-15T00:41:52Z

    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