Re: pg_createsubscriber --dry-run logging concerns

Hi Euler,

On Tue, Oct 7, 2025 at 7:06 AM Euler Taveira <euler(at)eulerto(dot)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