Re: [Proposal] Adding Log File Capability to pg_createsubscriber

On Mon, Mar 23, 2026 at 2:24 AM Chao Li <li.evan.chao@gmail.com> wrote:
+       /* Build timestamp directory path */
+       len = snprintf(logdir, MAXPGPATH, "%s/%s", log_basedir, timestamp);
+
+       if (len >= MAXPGPATH)
+               pg_fatal("directory path for log files, %s/%s, is too long",
+                                logdir, timestamp);
```
> In the pg_fatal call, I believe logdir should be log_basedir.
We are writing into logdir, and the if will be true only if it is too long.
Hence we should be checking logdir.

> The biggest problem I see with this patch is here.
internal_log_file_write doesn’t handle “%m”. I think we can check the
implementation of pg_log_generic_v how %m is handled. The key code snippet
is:
```
> errno = save_errno;

> va_copy(ap2, ap);
> required_len = vsnprintf(NULL, 0, fmt, ap2) + 1;
> va_end(ap2);
```
> Where, vsnprintf points to pg_vsnprintf, and pg_vsnprintf calls dopr to
handle %m.
I have saved and restored errno similar to that. The code snippet you
pointed out is, as far as I understand, where they are calculating how much
space to allocate (including the \0 at the end). I think it will be handled
automatically as long as errno is not overwritten - which it will now be.
Thank you!

>The other problem is, with internal_log_file_write, HINT, DETAIL prefix
are no longer printed, is that intentional?
I could add a switch-case to print it out. Is that important? What do you
think?

I have fixed the rest of your suggestions. Thank you, Chao Li!

On Mon, Mar 23, 2026 at 5:55 AM shveta malik <shveta.malik@gmail.com> wrote:

> We can get rid of below alignment related changes in unrelated test parts.
They are added when I run pgperltidy. Anyone else trying to change the file
after this would see the same thing if we don't change it. Should I move it
into another patch?

I have fixed the rest of it. Thank you, Shveta Malik!

On Mon, Mar 23, 2026 at 5:55 AM Kuroda, Hayato/黒田 隼人 <
kuroda.hayato@fujitsu.com> wrote:

> 01.
> Found that pg_log_generic_v() has some prefix but
> internal_log_file_write() does not.
> It means output strings are not the same. For example, on terminal:
>
> ```
> pg_createsubscriber: error: standby server is running
> pg_createsubscriber: hint: Stop the standby server and try again.
> ```
>
> But on log file:
> ```
> standby server is running
> Stop the standby server and try again.
> ```
>
> It's because pg_log_generic_v() has the format like below. I.e., the
> program name
> is printed at the begining, and some prefix also exists in some cases.
>
>         ${program name}: {error: |warning: |detail: |hint: } content
>
> I cannot find such a difference on pg_upgrade: no prefix exists in any
> cases.
> So, what should be here? My preference is to basically follow
> pg_log_generic_v()
> But remove the program name. How about others?
>

I haven't changed the output of pg_log_generic_v() yet. Shall I add the
prefix to the output? I have done the rest of your suggestions. Thank you!

Regards,
Gyan Sreejith