Re: pg_createsubscriber --dry-run logging concerns

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