RE: [Proposal] Adding Log File Capability to pg_createsubscriber

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Gyan Sreejith' <gyan.sreejith@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, shveta malik <shveta.malik@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, vignesh C <vignesh21@gmail.com>, Euler Taveira <euler@eulerto.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Peter Smith <smithpb2250@gmail.com>
Date: 2026-03-24T10:06:08Z
Lists: pgsql-hackers

Attachments

Dear Gyan,

Thanks for updating the patch!

> I haven't changed the output of pg_log_generic_v() yet. Shall I add the prefix to the output?

Since pg_log_generic_v() is the common function used even by others, we should
not fix it. Let's fix internal_log_file_write() instead.

Further comments:

01.
```
+               setvbuf(fh, NULL, PG_IOLBF, 0);
```

I think setvbuf is not needed.

IIUC, setvbuf() controls when messages like printf() are flushed from to the
stdout/stderr/files and so on. In logfile_open(), we are setting that output
messages should be flushed once per new lines. Syslogger process does not call
fflush() frequently thus it seems to be controlled by the library call.
However, pg_createsubscriber always does the fflush() every time, no need to
specify when the buffered strings are flushed on the file.

Also note that pg_upgrade does not call the library function.

02.
```
+               /* use CRLF line endings on Windows */
+               _setmode(_fileno(fh), _O_TEXT);
```

_setmode() is called in win32 system. According to [2] and code comment, it's
used to open the file as TEXT mode for unifying the new-line character to CR-LF.

However, postgres has already been done in fopen(). The function is overwritten
to pgwin32_fopen()->pgwin32_open(), and there we already specify to open with
the _O_TEXT mode. I think no need to do in the pg_createsubscriber.c.

Also note that pg_upgrade does not call the library function.

03.
If we fix above and comments from Amit [1], logfile_open() is not completely
different function from the syslogger.c. We can remove the comment atop the
function.

04.
As I told in [2], no need to do fclose() the file in the cleanup function.

I created a top-up patch set which addressed all comments from me and others.
See attached.

[1]: https://www.postgresql.org/message-id/CAA4eK1KHZraUvb5U5QdMEqiCAWhGPAnStun88ZR0qFunRZu9uQ%40mail.gmail.com
[2]: https://www.postgresql.org/message-id/OS9PR01MB12149F1AD9C79A2644753A18AF548A%40OS9PR01MB12149.jpnprd01.prod.outlook.com

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. pg_createsubscriber: Don't use MAXPGPATH

  2. pg_createsubscriber: Remove separate logfile_open() function

  3. pg_createsubscriber: Use logging.c log file callback

  4. Add log file support to logging.c

  5. pg_createsubscriber: Add -l/--logdir option to redirect output to files.

  6. pg_createsubscriber: Introduce module-specific logging functions.

  7. Fix another buglet in archive_waldump.c.