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/
