Обсуждение: pg_createsubscriber - more logging to say if there are no pubs to drop
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
Вложения
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
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
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
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
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
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
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