public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nisha Moond <[email protected]>
To: Gyan Sreejith <[email protected]>
Cc: Kuroda, Hayato/黒田 隼人 <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Subject: Re: [Proposal] Adding Log File Capability to pg_createsubscriber
Date: Fri, 20 Mar 2026 16:00:03 +0530
Message-ID: <CABdArM54_cVUMfE2DGSnJfDBZO1hmaVb82Z4b+1t0fp+xqXaMQ@mail.gmail.com> (raw)
In-Reply-To: <CAEqnbaU=EgqhxEx0ig4TdY8pdt0Vn+vmCJBuoORRHOzovW9dWA@mail.gmail.com>
References: <CAEqnbaUthOQARV1dscGvB_EsqC-YfxiM6rWkVDHc+G+f4oSUHw@mail.gmail.com>
<CAHut+PvizcpeHA1Twf_hwe=wANQ1LV5zY6_q+39gTFJc7+bCKg@mail.gmail.com>
<CAEqnbaVNZYbB_YufchM49d=XC1ZGrVV8ikCPmGotWoCZASY3Uw@mail.gmail.com>
<CAEqnbaV8QMbXZtt25QtGUPAaQZnD-B0HniFSroapMh+QTZgKsQ@mail.gmail.com>
<OSCPR01MB14966FD0961F512B29BD46D6BF5AAA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<CALDaNm0gX2D6fD5yur-R5gagA7+AfLmLZU5Z8+tgjo61b-Y01w@mail.gmail.com>
<[email protected]>
<CAA4eK1+2PYU-y2weY_zFeysP8Ux45iBNdaYMZuJtMTqXwCxawQ@mail.gmail.com>
<CAEqnbaVVp_g1m1nhOBZjtiAz5W-xwPQCmWH4hjYYehA+ktYg9Q@mail.gmail.com>
<CALDaNm2T__Uha1fn274bP3jKDSwm37ewWYvA+vOo75FTT2t3SA@mail.gmail.com>
<CALDaNm1fF6yBa2PCv9gO_MVcjKLQFgi9eAL=g5cU=xtbOr=qZw@mail.gmail.com>
<CAEqnbaUa27R9OcVDHkNuwsZ1uCpy8sTXpH78N1ug0kj60mnchw@mail.gmail.com>
<CALDaNm1Mj+eNVETmYAYd4ojrbTbQU5iCX20Fso24r2VuXF69AQ@mail.gmail.com>
<CAEqnbaUWSPGHLL2nuyNHVKU0TB7uGBx9w0NvnisWFyf5TtwRTQ@mail.gmail.com>
<CALDaNm3Dj7WRdfD+fQ86XzS07vewaFSWx--kB5689_sC7rt1Uw@mail.gmail.com>
<CAEqnbaVgFU2Pr=xhhDmA=sK7XPBDBxECovqbSht91ZbHmnteUg@mail.gmail.com>
<CALDaNm17ujrJ2xHyz6r81WAiFUs38RcT8D5ebdxssASpGko0HA@mail.gmail.com>
<CAEqnbaUbVgGMXBNFbk=sqFJT=_TkAUYGYsFHJKFw5K492M_B=A@mail.gmail.com>
<CALDaNm2SYq_dO7qXHn+BnxmxvmBupH6tofUnx4DKC09h5xfAyQ@mail.gmail.com>
<CAEqnbaWjoSby+_FQOKqTiDwf9AsWVjcGzRn-BQtdivC8xn0ADw@mail.gmail.com>
<[email protected]>
<CAA4eK1+jd6zH-LL8FOrw01chrNetRoZGKXSqV7GNo_--UXYc_w@mail.gmail.com>
<CAEqnbaU50vLy031AbvmfXJ3_qv9iS4pVaMdmsnmpF87r=EqW7Q@mail.gmail.com>
<CALDaNm1P5pZ2tTcaVRvjUAXLAyT7S18GPwp-4bc0xKXKZoXi0w@mail.gmail.com>
<CAEqnbaWxYp7TbnhKfkjW8c=jRPARsQ1on86bt_pcqvK9vCtLhQ@mail.gmail.com>
<CANhcyEWaCV7B54oOhuE_BhVCF887h23jKmkHDDWvJRKZ=pBbBQ@mail.gmail.com>
<CAEqnbaVRQwPNYqXVJu3YmPiS0d-kG00H81fEBR8ZULP0GJrp7Q@mail.gmail.com>
<CAA4eK1K1GLKiuFmcpcKJzhS-o2o2-aKEuaGWmHz-MfJhttaKFg@mail.gmail.com>
<OS9PR01MB1214902CC518A8102333FA88DF54EA@OS9PR01MB12149.jpnprd01.prod.outlook.com>
<OSOPR01MB1215385EAC6A76650444E40B0F54FA@OSOPR01MB12153.jpnprd01.prod.outlook.com>
<CAEqnbaU=EgqhxEx0ig4TdY8pdt0Vn+vmCJBuoORRHOzovW9dWA@mail.gmail.com>
On Thu, Mar 19, 2026 at 11:59 PM Gyan Sreejith <[email protected]> wrote:
>
> Thank you, Kuroda-san, for all the work.
> I have made a small change - I added a pg_free() to free internal_log_file.
>>
>>
Hi Gyan,
I reviewed/tested the patches, please find below comments for v13-002 patch -
File: pg_createsubscriber.c
1)
+
+ if ((internal_log_file_fp = logfile_open(internal_log_file, "a")) == NULL)
+ pg_fatal("could not open log file \"%s\": %m", internal_log_file);
+
IIUC, the pg_fatal() call here seems unreachable as the function
logfile_open() itself calls pg_fatal() and exits if it fails to open
the file.
I think it should just be -
internal_log_file_fp = logfile_open(internal_log_file, "a");
Please correct me if I'm missing something.
~~~
2)
+ if (opt->log_dir != NULL)
+ out_file = psprintf("%s/%s/%s.log", opt->log_dir, log_timestamp,
SERVER_LOG_FILE_NAME);
+ else
+ out_file = DEVNULL;
+
+ cmd_str = psprintf("\"%s\" -D \"%s\" >> \"%s\"", pg_resetwal_path,
+ subscriber_dir, out_file);
Similar to internal_log_file, why are 'out_file' and 'cmd_str' above not freed?
~~~
3) Typo - extra blank line after va_end(args);
@@ -205,10 +243,11 @@ pg_fatal(const char *pg_restrict fmt,...)
va_start(args, fmt);
- pg_log_generic_v(PG_LOG_ERROR, PG_LOG_PRIMARY, fmt, args);
+ pg_createsub_log_v(PG_LOG_ERROR, PG_LOG_PRIMARY, fmt, args);
va_end(args);
+
exit(1);
~~~
4) File: t/040_pg_createsubscriber.pl
+is( scalar(@server_log_files), 1, "
+ pg_createsubscriber_server.log file was created");
...
...
+is( scalar(@internal_log_files), 1, "
+ pg_createsubscriber_internal.log file was created");
Above introduces newlines in result log, test 29 and 32 looks like -
[14:46:59.071](0.639s) ok 28 - run pg_createsubscriber --dry-run on node S
[14:46:59.071](0.000s) ok 29 -
[14:46:59.071](0.000s) # pg_createsubscriber_server.log file was created
[14:46:59.071](0.000s) ok 30 - pg_createsubscriber_server.log file not empty
[14:46:59.071](0.000s) ok 31 - server reached consistent recovery state
[14:46:59.072](0.000s) ok 32 -
[14:46:59.072](0.000s) # pg_createsubscriber_internal.log file was created
[14:46:59.072](0.000s) ok 33 - pg_createsubscriber_internal.log file not empty
These should be single-line results similar to others.
--
Thanks,
Nisha
view thread (65+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [Proposal] Adding Log File Capability to pg_createsubscriber
In-Reply-To: <CABdArM54_cVUMfE2DGSnJfDBZO1hmaVb82Z4b+1t0fp+xqXaMQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox