Horton

Thread

Commits

  1. Split dry-run messages into primary and detail

  2. Log a note at program start when running in dry-run mode

  3. Use USECS_PER_SEC from datatype/timestamp.h

  4. Remove WaitPMResult enum in pg_createsubscriber

  5. pg_createsubscriber: reword dry-run log messages

  1. pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-01T02:29:56Z

    Lately, I've been reviewing some pg_createsubscriber patches and have
been tricked by some of the logging.

The pg_createsubscriber has a '--dry-run' option/mode [1]

----------
--dry-run
Do everything except actually modifying the target directory.
----------

I've noticed that the logging in '--dry-run' mode is indistinguishable
from the logging of "normal" run, although functions like
create_publication(), drop_publication(), etc, are NOP in '--dry-run'
mode, because the actual command execution code is skipped. e.g.

if (!dry_run)
{
  res = PQexec(conn, str->data);
  ...
}

~~~

IMO, it's not good to fool people into thinking something has happened
when in fact nothing happened at all. I think the logging of this tool
should be much more explicit wrt when it is/isn't in dry-run mode.
Perhaps like this:

NORMAL
pg_log_info("creating publication \"%s\" in database \"%s\"", ...)

DRY-RUN
pg_log_info("[dry-run] would create publication \"%s\" in database \"%s\"", ...)

~~~

Thoughts?

======
[1] https://www.postgresql.org/docs/current/app-pgcreatesubscriber.html

Kind Regards,
Peter Smith.
Fujitsu Australia

  2. RE: pg_createsubscriber --dry-run logging concerns

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-10-01T03:09:26Z

    Dear Peter,

> IMO, it's not good to fool people into thinking something has happened
> when in fact nothing happened at all. I think the logging of this tool
> should be much more explicit wrt when it is/isn't in dry-run mode.
> Perhaps like this:
> 
> NORMAL
> pg_log_info("creating publication \"%s\" in database \"%s\"", ...)
> 
> DRY-RUN
> pg_log_info("[dry-run] would create publication \"%s\" in database \"%s\"", ...)

Per my understanding, almost all the output must be adjusted based on the mode, right?
I feel it introduces a burden.
Can we solve the issue if we print additional message at the beginning if the
command runs with dry-run mode?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  3. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-01T03:51:40Z

    On Wed, Oct 1, 2025 at 1:09 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Peter,
>
> > IMO, it's not good to fool people into thinking something has happened
> > when in fact nothing happened at all. I think the logging of this tool
> > should be much more explicit wrt when it is/isn't in dry-run mode.
> > Perhaps like this:
> >
> > NORMAL
> > pg_log_info("creating publication \"%s\" in database \"%s\"", ...)
> >
> > DRY-RUN
> > pg_log_info("[dry-run] would create publication \"%s\" in database \"%s\"", ...)
>
> Per my understanding, almost all the output must be adjusted based on the mode, right?
> I feel it introduces a burden.
> Can we solve the issue if we print additional message at the beginning if the
> command runs with dry-run mode?
>

Hi Kuroda-san,

Yes, that is one way. Something is better than nothing, at least...

I think that not *everything* in dry mode is fake; some of the logged
operations are real. So, it might be good if we can show fake ones
differently. e.g. It may not take much effort to introduce a wrapper
that inserts a prefix. Use this as a replacement for just a few
specific info logs.

(code below may not work; it's just for illustrative purposes)

#define pg_log_info_checkdry(...) do {\
  if (dry_run)\
    pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]" __VA_ARGS__);\
  else;\
    pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
  } while (0);

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

  4. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-01T10:37:20Z

    On 2025-Oct-01, Peter Smith wrote:

> (code below may not work; it's just for illustrative purposes)
> 
> #define pg_log_info_checkdry(...) do {\
>   if (dry_run)\
>     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]" __VA_ARGS__);\
>   else;\
>     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
>   } while (0);

I like this kind of idea best.  However I think it might be better to do
it the other way around: have the normal pg_log_info() check dry_run,
and have a special one for the messages that are to be identical in
either mode.  I'm not sure how difficult this is to implement, though.

pg_subscriber is not the only program with a dry-run mode; it looks like
pg_archiveclean, pg_combinebackup, pg_resetwal, pg_rewind have one.  Is
it worth maybe doing something at the common/logging.c level rather than
specifically pg_createsubscriber?

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
[…] indem ich in meinem Leben oft an euch gedacht, euch glücklich zu machen. Seyd es!
A menudo he pensado en vosotros, en haceros felices. ¡Sedlo, pues!
		Heiligenstädter Testament, L. v. Beethoven, 1802
		https://de.wikisource.org/wiki/Heiligenstädter_Testament

  5. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-03T00:04:12Z

    On Wed, Oct 1, 2025 at 8:37 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-01, Peter Smith wrote:
>
> > (code below may not work; it's just for illustrative purposes)
> >
> > #define pg_log_info_checkdry(...) do {\
> >   if (dry_run)\
> >     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]" __VA_ARGS__);\
> >   else;\
> >     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
> >   } while (0);
>
> I like this kind of idea best.  However I think it might be better to do
> it the other way around: have the normal pg_log_info() check dry_run,
> and have a special one for the messages that are to be identical in
> either mode.  I'm not sure how difficult this is to implement, though.
>
> pg_subscriber is not the only program with a dry-run mode; it looks like
> pg_archiveclean, pg_combinebackup, pg_resetwal, pg_rewind have one.  Is
> it worth maybe doing something at the common/logging.c level rather than
> specifically pg_createsubscriber?
>

Hi Alvaro.

Thanks for the feedback and for pointing out the other tools that also
have a dr-run mode.

I did some quick back-of-the-envelope calculations to see what might
be involved.

======

For pg_create_subscriber, there are 38 info logs. Of those 38, I'd be
tempted to change only logs that say they are modifying/executing
something. There's ~15 of those:

pg_log_info("modifying system identifier of subscriber");
pg_log_info("running pg_resetwal on the subscriber");
pg_log_info("subscriber successfully changed the system identifier");
pg_log_info("use existing publication \"%s\" in database \"%s\"",
pg_log_info("create publication \"%s\" in database \"%s\"",
pg_log_info("create replication slot \"%s\" on publisher",
pg_log_info("dropping subscription \"%s\" in database \"%s\"",
pg_log_info("creating the replication slot \"%s\" in database \"%s\"",
pg_log_info("dropping the replication slot \"%s\" in database \"%s\"",
pg_log_info("creating publication \"%s\" in database \"%s\"",
pg_log_info("dropping publication \"%s\" in database \"%s\"",
pg_log_info("dropping all existing publications in database \"%s\"",
pg_log_info("creating subscription \"%s\" in database \"%s\"",
pg_log_info("setting the replication progress (node name \"%s\", LSN
%s) in database \"%s\"",
pg_log_info("enabling subscription \"%s\" in database \"%s\"",

======

For pg_archiveclean:

There only seems to be one logging for this dry run. Currently, it is
using debug logging like:

if (dry_run)
  pg_log_debug("would do XXX")
else
  pg_log_debug("doing XXX");

======

For pg_combinebackup:

This is the same as above; the differences for dry run logging are
already handled by having separate messages, and it uses debug logging
in many places with this pattern:

if (dry_run)
pg_log_debug("would do XXX")
else
    pg_log_debug("doing XXX");
======

For pg_resetwal:

This one is using a 'noupdate' flag instead of a 'dry_run' boolean.
And there doesn't seem to be any logging for this.

======

For pg_rewind:

Has 13 logs.  I'd be tempted to change maybe only a couple of these:

pg_rewind/pg_rewind.c: pg_log_info("creating backup label and updating
control file");
pg_rewind/pg_rewind.c: pg_log_info("executing \"%s\" for target server
to complete crash recovery",

======

I also found dry-run logs using the "would do XXX" pattern elsewhere,
in code like contrib/vacuumIo.c

//////

Summary

The idea to change the pg_log_info macro globally seems risky. There
are 400+ usages of this in the PG code, way beyond the scope of these
few tools that have a dry-run.

It seems that all the other --dry-run capable tools are using the pattern
if (dry_run)
pg_log_debug("would do XXX")
else
    pg_log_debug("doing XXX");

So, that makes pg_createsubscriber the odd man out. Instead of
introducing a new logging macro, perhaps it's better (for code
consistency) just to change pg_createsubscriber to use that same
logging pattern.

~~~

Kurdoa-san [1] was concerned it might be a big change burden to change
the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
that I'd be tempted to change in pg_createsubscriber.c; that's not
really such a burden.

OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
to indicate --dry-run might be useful. Even when run-time gives
different log messages, I think those other tools only show
differences when using DEBUG logging. Anybody not debugging might
otherwise have no idea that nothing actually happened.

~~

So, below is now my favoured solution:

1. Add an up-front info log to say when running in dry-run (add for
all tools that have --dry-run mode)

2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
consistent with all the other tools.
if (dry_run)
  pg_log_info("would do XXX")
else
  pg_log_info("doing XXX");

Thoughts?

======
[1] https://www.postgresql.org/message-id/OSCPR01MB149668DD062C1457FFAA44A28F5E6A%40OSCPR01MB14966.jpnprd01.prod.outlook.com

Kind Regards,
Peter Smith.
Fujitsu Australia

  6. Re: pg_createsubscriber --dry-run logging concerns

    Neeraj Shah <n4j@liamg.xyz> — 2025-10-03T17:50:35Z

    Hello Peter,

Your suggestion makes sense to me, if it's fine with you then I can submit
a patch for this change :).

[
So, below is now my favoured solution:

1. Add an up-front info log to say when running in dry-run (add for
all tools that have --dry-run mode)

2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
consistent with all the other tools.
if (dry_run)
  pg_log_info("would do XXX")
else
  pg_log_info("doing XXX");
]

Thanks,
Neeraj Shah
Nutanix Inc.


On Fri, Oct 3, 2025 at 5:34 AM Peter Smith <smithpb2250@gmail.com> wrote:

> On Wed, Oct 1, 2025 at 8:37 PM Álvaro Herrera <alvherre@kurilemu.de>
> wrote:
> >
> > On 2025-Oct-01, Peter Smith wrote:
> >
> > > (code below may not work; it's just for illustrative purposes)
> > >
> > > #define pg_log_info_checkdry(...) do {\
> > >   if (dry_run)\
> > >     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, "[dry-run NOP]"
> __VA_ARGS__);\
> > >   else;\
> > >     pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__);\
> > >   } while (0);
> >
> > I like this kind of idea best.  However I think it might be better to do
> > it the other way around: have the normal pg_log_info() check dry_run,
> > and have a special one for the messages that are to be identical in
> > either mode.  I'm not sure how difficult this is to implement, though.
> >
> > pg_subscriber is not the only program with a dry-run mode; it looks like
> > pg_archiveclean, pg_combinebackup, pg_resetwal, pg_rewind have one.  Is
> > it worth maybe doing something at the common/logging.c level rather than
> > specifically pg_createsubscriber?
> >
>
> Hi Alvaro.
>
> Thanks for the feedback and for pointing out the other tools that also
> have a dr-run mode.
>
> I did some quick back-of-the-envelope calculations to see what might
> be involved.
>
> ======
>
> For pg_create_subscriber, there are 38 info logs. Of those 38, I'd be
> tempted to change only logs that say they are modifying/executing
> something. There's ~15 of those:
>
> pg_log_info("modifying system identifier of subscriber");
> pg_log_info("running pg_resetwal on the subscriber");
> pg_log_info("subscriber successfully changed the system identifier");
> pg_log_info("use existing publication \"%s\" in database \"%s\"",
> pg_log_info("create publication \"%s\" in database \"%s\"",
> pg_log_info("create replication slot \"%s\" on publisher",
> pg_log_info("dropping subscription \"%s\" in database \"%s\"",
> pg_log_info("creating the replication slot \"%s\" in database \"%s\"",
> pg_log_info("dropping the replication slot \"%s\" in database \"%s\"",
> pg_log_info("creating publication \"%s\" in database \"%s\"",
> pg_log_info("dropping publication \"%s\" in database \"%s\"",
> pg_log_info("dropping all existing publications in database \"%s\"",
> pg_log_info("creating subscription \"%s\" in database \"%s\"",
> pg_log_info("setting the replication progress (node name \"%s\", LSN
> %s) in database \"%s\"",
> pg_log_info("enabling subscription \"%s\" in database \"%s\"",
>
> ======
>
> For pg_archiveclean:
>
> There only seems to be one logging for this dry run. Currently, it is
> using debug logging like:
>
> if (dry_run)
>   pg_log_debug("would do XXX")
> else
>   pg_log_debug("doing XXX");
>
> ======
>
> For pg_combinebackup:
>
> This is the same as above; the differences for dry run logging are
> already handled by having separate messages, and it uses debug logging
> in many places with this pattern:
>
> if (dry_run)
> pg_log_debug("would do XXX")
> else
>     pg_log_debug("doing XXX");
> ======
>
> For pg_resetwal:
>
> This one is using a 'noupdate' flag instead of a 'dry_run' boolean.
> And there doesn't seem to be any logging for this.
>
> ======
>
> For pg_rewind:
>
> Has 13 logs.  I'd be tempted to change maybe only a couple of these:
>
> pg_rewind/pg_rewind.c: pg_log_info("creating backup label and updating
> control file");
> pg_rewind/pg_rewind.c: pg_log_info("executing \"%s\" for target server
> to complete crash recovery",
>
> ======
>
> I also found dry-run logs using the "would do XXX" pattern elsewhere,
> in code like contrib/vacuumIo.c
>
> //////
>
> Summary
>
> The idea to change the pg_log_info macro globally seems risky. There
> are 400+ usages of this in the PG code, way beyond the scope of these
> few tools that have a dry-run.
>
> It seems that all the other --dry-run capable tools are using the pattern
> if (dry_run)
> pg_log_debug("would do XXX")
> else
>     pg_log_debug("doing XXX");
>
> So, that makes pg_createsubscriber the odd man out. Instead of
> introducing a new logging macro, perhaps it's better (for code
> consistency) just to change pg_createsubscriber to use that same
> logging pattern.
>
> ~~~
>
> Kurdoa-san [1] was concerned it might be a big change burden to change
> the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
> that I'd be tempted to change in pg_createsubscriber.c; that's not
> really such a burden.
>
> OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
> to indicate --dry-run might be useful. Even when run-time gives
> different log messages, I think those other tools only show
> differences when using DEBUG logging. Anybody not debugging might
> otherwise have no idea that nothing actually happened.
>
> ~~
>
> So, below is now my favoured solution:
>
> 1. Add an up-front info log to say when running in dry-run (add for
> all tools that have --dry-run mode)
>
> 2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
> consistent with all the other tools.
> if (dry_run)
>   pg_log_info("would do XXX")
> else
>   pg_log_info("doing XXX");
>
> Thoughts?
>
> ======
> [1]
> https://www.postgresql.org/message-id/OSCPR01MB149668DD062C1457FFAA44A28F5E6A%40OSCPR01MB14966.jpnprd01.prod.outlook.com
>
> Kind Regards,
> Peter Smith.
> Fujitsu Australia
>
>
>

  7. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-04T11:57:44Z

    On 2025-Oct-03, Peter Smith wrote:

> Summary
> 
> The idea to change the pg_log_info macro globally seems risky. There
> are 400+ usages of this in the PG code, way beyond the scope of these
> few tools that have a dry-run.

Ok.

> So, that makes pg_createsubscriber the odd man out. Instead of
> introducing a new logging macro, perhaps it's better (for code
> consistency) just to change pg_createsubscriber to use that same
> logging pattern.

Sure, let's go that way.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

  8. Re: pg_createsubscriber --dry-run logging concerns

    Euler Taveira <euler@eulerto.com> — 2025-10-06T20:06:01Z

    On Thu, Oct 2, 2025, at 9:04 PM, Peter Smith wrote:
> Summary
>
> The idea to change the pg_log_info macro globally seems risky. There
> are 400+ usages of this in the PG code, way beyond the scope of these
> few tools that have a dry-run.
>
> It seems that all the other --dry-run capable tools are using the pattern
> if (dry_run)
> pg_log_debug("would do XXX")
> else
>     pg_log_debug("doing XXX");
>
> So, that makes pg_createsubscriber the odd man out. Instead of
> introducing a new logging macro, perhaps it's better (for code
> consistency) just to change pg_createsubscriber to use that same
> logging pattern.
>

What do you mean by "use the same logging pattern"? During development we
discussed if it is worth to double the number of messages to have accurate log
messages in dry run mode but decided it isn't.

I didn't understand all details of your proposal but I would like to say that
I'm against changing the 2 level log messages. Sometimes we just want a list of
the steps with the exact object names (hence, --verbose) instead of a bunch of
additional debug messages that exposes the implementation details (hence,
--verbose --verbose).

> Kurdoa-san [1] was concerned it might be a big change burden to change
> the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
> that I'd be tempted to change in pg_createsubscriber.c; that's not
> really such a burden.
>
> OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
> to indicate --dry-run might be useful. Even when run-time gives
> different log messages, I think those other tools only show
> differences when using DEBUG logging. Anybody not debugging might
> otherwise have no idea that nothing actually happened.
>

I agree that it seems confusing if you are not the one that wrote the command
line. Maybe it would be less confusing if we have a log message at the
beginning saying "pg_createsubscriber is in dry run mode".

> So, below is now my favoured solution:
>
> 1. Add an up-front info log to say when running in dry-run (add for
> all tools that have --dry-run mode)
>
> 2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
> consistent with all the other tools.
> if (dry_run)
>   pg_log_info("would do XXX")
> else
>   pg_log_info("doing XXX");
>

I particularly think a prefix increases the translation effort. As Alvaro said
if you want to propose a prefix feature, it should cover the other tools that
use the logging module too.

Since you are improving messages, I suggest 2 changes:

pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"
pg_createsubscriber: create replication slot "pg_createsubscriber_16384_710455e1" on publisher

to

pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"

because it is duplicated.

Don't refer to the database name for physical replication slot

pg_createsubscriber: dropping the replication slot "primaryslot" in database "bench1"

Maybe replace "in database foo" with "on primary".


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

  9. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-08T09:03:26Z

    Hi Euler,

On Tue, Oct 7, 2025 at 7:06 AM Euler Taveira <euler@eulerto.com> wrote:
>
> On Thu, Oct 2, 2025, at 9:04 PM, Peter Smith wrote:
> > Summary
> >
> > The idea to change the pg_log_info macro globally seems risky. There
> > are 400+ usages of this in the PG code, way beyond the scope of these
> > few tools that have a dry-run.
> >
> > It seems that all the other --dry-run capable tools are using the pattern
> > if (dry_run)
> > pg_log_debug("would do XXX")
> > else
> >     pg_log_debug("doing XXX");
> >
> > So, that makes pg_createsubscriber the odd man out. Instead of
> > introducing a new logging macro, perhaps it's better (for code
> > consistency) just to change pg_createsubscriber to use that same
> > logging pattern.
> >
>
> What do you mean by "use the same logging pattern"? During development we
> discussed if it is worth to double the number of messages to have accurate log
> messages in dry run mode but decided it isn't.
>

Yes, one part I proposed is to do exactly that. i.e. double-up on some
messages (about a dozen of them), because that is what the other tools
that have '--dry-run' mode are doing. They have messages like:
"creating xxx" and related one for --dry-run that says "would create xxx"

So I was proposing to do the same, consistent code pattern with the other tools.

> I didn't understand all details of your proposal but I would like to say that
> I'm against changing the 2 level log messages. Sometimes we just want a list of
> the steps with the exact object names (hence, --verbose) instead of a bunch of
> additional debug messages that exposes the implementation details (hence,
> --verbose --verbose).

Yeah, I understand that the other places (like pg_combinebackup.c) are
using pg_log_debug instead of pg_log_info, so perhaps your point is
that although it was OK to do this in debug, you think doing the same
for pg_log_info is a bridge too far?

I am not wedded to doing this double-messaging... if people feel just
the one-time logging at the beginning is enough, then that is OK by
me.

>
> > Kurdoa-san [1] was concerned it might be a big change burden to change
> > the pg_createsubscriber logs, but AFAICT there are only ~15/38 places
> > that I'd be tempted to change in pg_createsubscriber.c; that's not
> > really such a burden.
> >
> > OTOH, I do think Kuroda-san's idea [1] of giving some up-front logging
> > to indicate --dry-run might be useful. Even when run-time gives
> > different log messages, I think those other tools only show
> > differences when using DEBUG logging. Anybody not debugging might
> > otherwise have no idea that nothing actually happened.
> >
>
> I agree that it seems confusing if you are not the one that wrote the command
> line. Maybe it would be less confusing if we have a log message at the
> beginning saying "pg_createsubscriber is in dry run mode".
>

Yes. So, please see the attached patch that implements this. And for
consistency, the change is repeated to all other tools that use
--dry-run mode.

> > So, below is now my favoured solution:
> >
> > 1. Add an up-front info log to say when running in dry-run (add for
> > all tools that have --dry-run mode)
> >
> > 2. Modify ~15 places in pg_createsubscriber.c to use the code pattern
> > consistent with all the other tools.
> > if (dry_run)
> >   pg_log_info("would do XXX")
> > else
> >   pg_log_info("doing XXX");
> >
>
> I particularly think a prefix increases the translation effort. As Alvaro said
> if you want to propose a prefix feature, it should cover the other tools that
> use the logging module too.
>

There are no plans to have a prefix feature. That was an initial
thought bubble, but after I saw how all the other tools that have
--dry-run just have pairs of logging, I dropped the prefix idea.

> Since you are improving messages, I suggest 2 changes:
>
> pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"
> pg_createsubscriber: create replication slot "pg_createsubscriber_16384_710455e1" on publisher
>
> to
>
> pg_createsubscriber: creating the replication slot "pg_createsubscriber_16384_710455e1" in database "bench1"
>
> because it is duplicated.
>
> Don't refer to the database name for physical replication slot
>
> pg_createsubscriber: dropping the replication slot "primaryslot" in database "bench1"
>
> Maybe replace "in database foo" with "on primary".
>

Actually. I had already created another thread/patch about that
duplicate logging issue [1].

As for all the other suggestions, I'd rather keep this thread scope
focused on the '--dry-run' logging issue, and not conflate it with all
those other logging problems, which probably all deserve their own
threads/patches.

======
[1] https://www.postgresql.org/message-id/CAHut+Pv7qDvLbDgc9PQGhULT3rPXTxdu_=w+iW-kMs+zPADR+w@mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

  10. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-08T10:04:04Z

    On 2025-Oct-08, Peter Smith wrote:

> I am not wedded to doing this double-messaging... if people feel just
> the one-time logging at the beginning is enough, then that is OK by
> me.

Some other tools (not Postgres ones) do that and it always makes me
nervous, because I can never be sure which parts were actually done and
which parts are only dry-run trials.  I'd rather stay away from that
approach.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"¿Cómo puedes confiar en algo que pagas y que no ves,
y no confiar en algo que te dan y te lo muestran?" (Germán Poo)

  11. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-08T22:42:10Z

    On Sat, Oct 4, 2025 at 3:50 AM Neeraj Shah <n4j@liamg.xyz> wrote:
>
> Hello Peter,
>
> Your suggestion makes sense to me, if it's fine with you then I can submit a patch for this change :).
>

Thanks, but I already had some local patches ready to go.... I was
just waiting for more feedback before posting them.

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

  12. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-09T00:55:55Z

    On Wed, Oct 8, 2025 at 9:04 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-08, Peter Smith wrote:
>
> > I am not wedded to doing this double-messaging... if people feel just
> > the one-time logging at the beginning is enough, then that is OK by
> > me.
>
> Some other tools (not Postgres ones) do that and it always makes me
> nervous, because I can never be sure which parts were actually done and
> which parts are only dry-run trials.  I'd rather stay away from that
> approach.
>

OK. Here are v2 patches to implement both ways. You can pick one or both.

0001 - a beginning log to say if the tool is in dry-run mode
0002 - alternate pg_log_info messages for pg_createsubscriber, when in
dry-run mode

~

Note: there might be a clash with another thread [1] if that gets
committed soon; I will deal with any rebase if/when it is needed.

======
[1] https://www.postgresql.org/message-id/flat/CAHut%2BPv7qDvLbDgc9PQGhULT3rPXTxdu_%3Dw%2BiW-kMs%2BzPADR%2Bw%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

  13. Re: pg_createsubscriber --dry-run logging concerns

    Chao Li <li.evan.chao@gmail.com> — 2025-10-09T05:38:27Z

    I think is patch is helpful. A few comments:

> On Oct 9, 2025, at 08:55, Peter Smith <smithpb2250@gmail.com> wrote:
> 
> <v2-0001-log-to-say-command-is-executing-in-dry-run-mode.patch><v2-0002-add-different-dry-run-logging-for-pg_createsubscr.patch>



1 - 0001
```
+	if (dryrun)
+	{
+		pg_log_info("-----------------------------------------------------");
+		pg_log_info("pg_archivecleanup is executing in '--dry-run' mode.");
+		pg_log_info("No files will be removed.");
+		pg_log_info("-----------------------------------------------------");
+	}
```

Putting the program name in log message feels redundant, because pg_log_info() may already prefixes logs with program name. But I like the separator lines that make it stand out visually in logs. So this log can be simplified as:

```
if (dryrun)
{
    pg_log_info("------------------------------------------------------------");
    pg_log_info("Running in dry-run mode; no files will be removed.");
    pg_log_info("------------------------------------------------------------");
}
```

This comment applies to rest of changes in 0001.

2 - 0002
```
-	if (!dry_run)
+	if (dry_run)
+		pg_log_info("in dry-run mode, otherwise system identifier would be %" PRIu64 " on subscriber",
+					cf->system_identifier);
```

I think this log message can be simplified as:

```
pg_log_info("dry-run: system identifier would be %" PRIu64 " on subscriber",
            cf->system_identifier);
```

As a general comment, I think “in dry-run mode” is a little long-winded, we can just put “dry-run:”, and “otherwise” seems not needed because “dry-run” has clearly indicated nothing would actually happen. This comment applies to all changes in pg_createsubscriber.c of 0002.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  14. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-10-09T08:05:24Z

    On Thu, Oct 9, 2025 at 4:38 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> I think is patch is helpful. A few comments:
>
> On Oct 9, 2025, at 08:55, Peter Smith <smithpb2250@gmail.com> wrote:
>
> <v2-0001-log-to-say-command-is-executing-in-dry-run-mode.patch><v2-0002-add-different-dry-run-logging-for-pg_createsubscr.patch>
>
>
>
> 1 - 0001
> ```
> + if (dryrun)
> + {
> + pg_log_info("-----------------------------------------------------");
> + pg_log_info("pg_archivecleanup is executing in '--dry-run' mode.");
> + pg_log_info("No files will be removed.");
> + pg_log_info("-----------------------------------------------------");
> + }
> ```
>
> Putting the program name in log message feels redundant, because pg_log_info() may already prefixes logs with program name. But I like the separator lines that make it stand out visually in logs. So this log can be simplified as:
>
> ```

Fair point. I removed the tool name from the message.

> 2 - 0002
> ```
> - if (!dry_run)
> + if (dry_run)
> + pg_log_info("in dry-run mode, otherwise system identifier would be %" PRIu64 " on subscriber",
> + cf->system_identifier);
> ```
>
> I think this log message can be simplified as:
>
> ```
> pg_log_info("dry-run: system identifier would be %" PRIu64 " on subscriber",
>             cf->system_identifier);
> ```
>
> As a general comment, I think “in dry-run mode” is a little long-winded, we can just put “dry-run:”, and “otherwise” seems not needed because “dry-run” has clearly indicated nothing would actually happen. This comment applies to all changes in pg_createsubscriber.c of 0002.
>

OK. Updated as suggested.

~~

The anticipated rebase was necessary, too.

Please see the v3 patches.

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

  15. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-31T18:02:32Z

    On 2025-Oct-09, Peter Smith wrote:

> Please see the v3 patches.

Okay, I have pushed 0002 to 18 and master.  I wanted to backpatch to 17
but there are too many conflicts there.

I'm not opposed to 0001 (to master only), but I think the lines of
dashes are a little excessively noisy.  Are there other opinions on
that?  Note that vacuumlo also has it, with no dashes.

I'll mark the commitfest item for version 19.

While looking this over, I noticed that we define USEC_PER_SEC, but why?
We already have USECS_PER_SEC, so let's use that.  And WaitPMResult
seems an overgrown boolean, so how about we remove it?  Patch for these
things attached.  Thoughts?

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Thou shalt study thy libraries and strive not to reinvent them without
cause, that thy code may be short and readable and thy days pleasant
and productive. (7th Commandment for C Programmers)

  16. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-11-04T01:16:36Z

    On Sat, Nov 1, 2025 at 5:02 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Oct-09, Peter Smith wrote:
>
> > Please see the v3 patches.
>
> Okay, I have pushed 0002 to 18 and master.  I wanted to backpatch to 17
> but there are too many conflicts there.
>

Thanks for pushing!

> I'm not opposed to 0001 (to master only), but I think the lines of
> dashes are a little excessively noisy.  Are there other opinions on
> that?  Note that vacuumlo also has it, with no dashes.

It was intentionally "noisy" with dashes to make it impossible to
accidentally overlook dry-run mode in the logs. But if that's contrary
to the way Postgres does things I am fine if you want to remove the
dashes.

>
> I'll mark the commitfest item for version 19.
>

> While looking this over, I noticed that we define USEC_PER_SEC, but why?
> We already have USECS_PER_SEC, so let's use that.  And WaitPMResult
> seems an overgrown boolean, so how about we remove it?  Patch for these
> things attached.  Thoughts?
>

I took a quick look at your follow-up patches and below are some comments:

//////////
Patch 0001 - USECS_PER_SEC
//////////

+1 to use USECS_PER_SEC from timestamps.h

1.
-#define USEC_PER_SEC 1000000
-
-#define WAITS_PER_SEC 10 /* should divide USEC_PER_SEC evenly */
+#define WAITS_PER_SEC 10 /* should divide USECS_PER_SEC evenly */

 static bool do_wait = true;
 static int wait_seconds = DEFAULT_WAIT;
@@ -594,6 +593,7 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint)
 {
  int i;

+ static_assert(USECS_PER_SEC % WAITS_PER_SEC == 0);

Maybe that static assert might be better placed adjacent to the
#define to enforce that comment about "divide evenly".

SUGGESTION
#define WAITS_PER_SEC 10

/* WAITS_PER_SEC should divide USECS_PER_SEC evenly */
StatiAssertDecl(USECS_PER_SEC % WAITS_PER_SEC == 0);

//////////
Patch 0002 -- boolean
//////////

LGTM.

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

  17. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-04T09:59:27Z

    On 2025-Nov-04, Peter Smith wrote:

> On Sat, Nov 1, 2025 at 5:02 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:

> > I'm not opposed to 0001 (to master only), but I think the lines of
> > dashes are a little excessively noisy.  Are there other opinions on
> > that?  Note that vacuumlo also has it, with no dashes.
> 
> It was intentionally "noisy" with dashes to make it impossible to
> accidentally overlook dry-run mode in the logs. But if that's contrary
> to the way Postgres does things I am fine if you want to remove the
> dashes.

Yeah, I don't know if it's The Postgres Way or just accidental, but I'd
rather stay away from that kind of noise.  Also, from a i18n point of
view, the lines of dashes (as implemented here) would get in the message
catalogs, which nobody will appreciate.  Please do make the messages a
single string rather than two separate strings though, perhaps with an
embedded newline if more than one line of output is needed.

> I took a quick look at your follow-up patches and below are some comments:

Thanks for looking at them.  I pushed 0002; I'm waiting on the CI run
https://cirrus-ci.com/build/5618025065349120
to push 0001.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

  18. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-11-10T04:00:03Z

    On Tue, Nov 4, 2025 at 10:23 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Nov-04, Peter Smith wrote:
>
> > On Sat, Nov 1, 2025 at 5:02 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> > > I'm not opposed to 0001 (to master only), but I think the lines of
> > > dashes are a little excessively noisy.  Are there other opinions on
> > > that?  Note that vacuumlo also has it, with no dashes.
> >
> > It was intentionally "noisy" with dashes to make it impossible to
> > accidentally overlook dry-run mode in the logs. But if that's contrary
> > to the way Postgres does things I am fine if you want to remove the
> > dashes.
>
> Yeah, I don't know if it's The Postgres Way or just accidental, but I'd
> rather stay away from that kind of noise.  Also, from a i18n point of
> view, the lines of dashes (as implemented here) would get in the message
> catalogs, which nobody will appreciate.  Please do make the messages a
> single string rather than two separate strings though, perhaps with an
> embedded newline if more than one line of output is needed.
>

Hi Alvaro,

Here is patch v4-0001 modified as requested:
- dashes are removed
- the message is a single string

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

  19. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-11T09:17:41Z

    On 2025-Nov-10, Peter Smith wrote:

> Hi Alvaro,
> 
> Here is patch v4-0001 modified as requested:
> - dashes are removed
> - the message is a single string

Okay, thanks.  I split the strings in two lines, as we customarily do
when they contain embedded newlines.  I also noticed pg_resetwal uses
stdout rather than stderr and set out to change it, because I don't
think it's sensible to have one program behave one way (print to stdout)
when all others behave in another (to stderr).  I wrote a commit message
and was about ready to push.

However, I then found out that the reason you used stdout instead of
stderr in pg_resetwal is that with the latter, tests fail all over the
place because of pg_resetwal -n being used for pg_upgrade internally via
popen(), and making it write to stderr results in confusing pg_upgrade
output as well as test failures.  A very simple fix for this problem
would be, of course, to add " 2>/dev/null" to the popen call, but that
is not only cheating, it is also dangerous: if pg_resetwal ever finds
reason to complain, we won't get very good information because of that
redirection.

(I also don't think this line belongs in stdout, in case you're thinking
of changing it in the other direction for all other programs.)

Maybe we should add a -q,--silent mode that suppresses the "Running in
dry-run mode" line.  I do wonder if this is getting too far into the
weeds for such a small thing.  I won't blame you if you want to just
drop this whole idea, but I also won't stop you if you want to introduce
--silent.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No necesitamos banderas
 No reconocemos fronteras"                  (Jorge González)

  20. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-11-12T02:01:24Z

    On Tue, Nov 11, 2025 at 8:17 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Nov-10, Peter Smith wrote:
>
> > Hi Alvaro,
> >
> > Here is patch v4-0001 modified as requested:
> > - dashes are removed
> > - the message is a single string
>
> Okay, thanks.  I split the strings in two lines, as we customarily do
> when they contain embedded newlines.  I also noticed pg_resetwal uses
> stdout rather than stderr and set out to change it, because I don't
> think it's sensible to have one program behave one way (print to stdout)
> when all others behave in another (to stderr).  I wrote a commit message
> and was about ready to push.
>
> However, I then found out that the reason you used stdout instead of
> stderr in pg_resetwal is that with the latter, tests fail all over the
> place because of pg_resetwal -n being used for pg_upgrade internally via
> popen(), and making it write to stderr results in confusing pg_upgrade
> output as well as test failures.  A very simple fix for this problem

Right.

> would be, of course, to add " 2>/dev/null" to the popen call, but that
> is not only cheating, it is also dangerous: if pg_resetwal ever finds
> reason to complain, we won't get very good information because of that
> redirection.
>
> (I also don't think this line belongs in stdout, in case you're thinking
> of changing it in the other direction for all other programs.)
>
> Maybe we should add a -q,--silent mode that suppresses the "Running in
> dry-run mode" line.  I do wonder if this is getting too far into the
> weeds for such a small thing.  I won't blame you if you want to just
> drop this whole idea, but I also won't stop you if you want to introduce
> --silent.
>

It is tempting to implement a "--silent" mode, but if I did that, I
would then feel obliged to document and test it. I don't want to go
further down this rabbit hole for what was originally supposed to be
trivial logging.

So, I am calling it quits for this 0001 patch.

Perhaps it's still of some use to push changes for everything except
the pg_resetwal? Or if you prefer to just abandon the whole patch,
that is OK too. Thanks for trying.

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

  21. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-18T15:39:05Z

    On 2025-Nov-12, Peter Smith wrote:

> It is tempting to implement a "--silent" mode, but if I did that, I
> would then feel obliged to document and test it. I don't want to go
> further down this rabbit hole for what was originally supposed to be
> trivial logging.

Yeah, I understand.

> So, I am calling it quits for this 0001 patch.
> 
> Perhaps it's still of some use to push changes for everything except
> the pg_resetwal? Or if you prefer to just abandon the whole patch,
> that is OK too. Thanks for trying.

Got it.  I pushed it for the other programs and marked the CF entry as
committed -- thanks for the patches and the discussion.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"I'm impressed how quickly you are fixing this obscure issue. I came from 
MS SQL and it would be hard for me to put into words how much of a better job
you all are doing on [PostgreSQL]."
 Steve Midgley, http://archives.postgresql.org/pgsql-sql/2008-08/msg00000.php

  22. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2025-11-18T21:35:16Z

    On Wed, Nov 19, 2025 at 2:39 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Nov-12, Peter Smith wrote:
>
> > It is tempting to implement a "--silent" mode, but if I did that, I
> > would then feel obliged to document and test it. I don't want to go
> > further down this rabbit hole for what was originally supposed to be
> > trivial logging.
>
> Yeah, I understand.
>
> > So, I am calling it quits for this 0001 patch.
> >
> > Perhaps it's still of some use to push changes for everything except
> > the pg_resetwal? Or if you prefer to just abandon the whole patch,
> > that is OK too. Thanks for trying.
>
> Got it.  I pushed it for the other programs and marked the CF entry as
> committed -- thanks for the patches and the discussion.

Thanks for pushing!

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

  23. Re: pg_createsubscriber --dry-run logging concerns

    Peter Eisentraut <peter@eisentraut.org> — 2026-06-29T13:56:35Z

    On 18.11.25 22:35, Peter Smith wrote:
> On Wed, Nov 19, 2025 at 2:39 AM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>
>> On 2025-Nov-12, Peter Smith wrote:
>>
>>> It is tempting to implement a "--silent" mode, but if I did that, I
>>> would then feel obliged to document and test it. I don't want to go
>>> further down this rabbit hole for what was originally supposed to be
>>> trivial logging.
>>
>> Yeah, I understand.
>>
>>> So, I am calling it quits for this 0001 patch.
>>>
>>> Perhaps it's still of some use to push changes for everything except
>>> the pg_resetwal? Or if you prefer to just abandon the whole patch,
>>> that is OK too. Thanks for trying.
>>
>> Got it.  I pushed it for the other programs and marked the CF entry as
>> committed -- thanks for the patches and the discussion.
> 
> Thanks for pushing!

I have a follow-up for this.  Earlier in this thread it was asked to 
combine the dry-run info message into one string with a newline.  But I 
think this doesn't really fit the message style guidelines.  I suggest 
to use a primary plus detail style here too.  How about the attached patch?

  24. Re: pg_createsubscriber --dry-run logging concerns

    Euler Taveira <euler@eulerto.com> — 2026-06-29T20:14:41Z

    On Mon, Jun 29, 2026, at 10:56 AM, Peter Eisentraut wrote:
>
> I have a follow-up for this.  Earlier in this thread it was asked to 
> combine the dry-run info message into one string with a newline.  But I 
> think this doesn't really fit the message style guidelines.  I suggest 
> to use a primary plus detail style here too.  How about the attached patch?
>

LGTM.


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

  25. Re: pg_createsubscriber --dry-run logging concerns

    Peter Smith <smithpb2250@gmail.com> — 2026-06-29T20:21:39Z

    On Tue, Jun 30, 2026 at 8:15 AM Euler Taveira <euler@eulerto.com> wrote:
>
> On Mon, Jun 29, 2026, at 10:56 AM, Peter Eisentraut wrote:
> >
> > I have a follow-up for this.  Earlier in this thread it was asked to
> > combine the dry-run info message into one string with a newline.  But I
> > think this doesn't really fit the message style guidelines.  I suggest
> > to use a primary plus detail style here too.  How about the attached patch?
> >
>
> LGTM.
>

Ditto.

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

  26. Re: pg_createsubscriber --dry-run logging concerns

    Álvaro Herrera <alvherre@kurilemu.de> — 2026-06-29T21:13:02Z

    On 2026-06-29, Peter Eisentraut wrote:

> I have a follow-up for this.  Earlier in this thread it was asked to 
> combine the dry-run info message into one string with a newline.  But I 
> think this doesn't really fit the message style guidelines.  I suggest 
> to use a primary plus detail style here too.  How about the attached patch?

Yeah, that works for me.  It makes me wonder what was I thinking when I requested that newline.

-- 
Álvaro Herrera

  27. Re: pg_createsubscriber --dry-run logging concerns

    Peter Eisentraut <peter@eisentraut.org> — 2026-07-01T08:21:50Z

    On 29.06.26 23:13, Álvaro Herrera wrote:
> On 2026-06-29, Peter Eisentraut wrote:
> 
>> I have a follow-up for this.  Earlier in this thread it was asked to
>> combine the dry-run info message into one string with a newline.  But I
>> think this doesn't really fit the message style guidelines.  I suggest
>> to use a primary plus detail style here too.  How about the attached patch?
> 
> Yeah, that works for me.  It makes me wonder what was I thinking when I requested that newline.

Ok, done.