From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Euler Taveira <euler(at)eulerto(dot)com>, "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-09 05:38:27 |
Message-ID: | 52A644E6-FB2B-4A07-AC48-CE51EF7CA8EF@gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I think is patch is helpful. A few comments:
> On Oct 9, 2025, at 08:55, Peter Smith <smithpb2250(at)gmail(dot)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/
From | Date | Subject | |
---|---|---|---|
Next Message | Hayato Kuroda (Fujitsu) | 2025-10-09 05:47:49 | RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE |
Previous Message | Fujii Masao | 2025-10-09 05:26:21 | Re: Invalid primary_slot_name triggers warnings in all processes on reload |