From: | "Euler Taveira" <euler(at)eulerto(dot)com> |
---|---|
To: | "Peter Smith" <smithpb2250(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
Cc: | "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pg_createsubscriber --dry-run logging concerns |
Date: | 2025-10-06 20:06:01 |
Message-ID: | b4556150-de5d-4196-ae24-25a4c01225eb@app.fastmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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/
From | Date | Subject | |
---|---|---|---|
Next Message | Joel Jacobson | 2025-10-06 20:11:21 | Re: Optimize LISTEN/NOTIFY |
Previous Message | Masahiko Sawada | 2025-10-06 20:00:10 | Re: Invalid pointer access in logical decoding after error |