MIME-Version: 1.0
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>
 <48261e54-c4f1-4bde-a4a9-4f3698a6b380@app.fastmail.com>
 <CAA4eK1+2PYU-y2weY_zFeysP8Ux45iBNdaYMZuJtMTqXwCxawQ@mail.gmail.com>
 <CAEqnbaVVp_g1m1nhOBZjtiAz5W-xwPQCmWH4hjYYehA+ktYg9Q@mail.gmail.com>
In-Reply-To: 
 <CAEqnbaVVp_g1m1nhOBZjtiAz5W-xwPQCmWH4hjYYehA+ktYg9Q@mail.gmail.com>
From: vignesh C <vignesh21@gmail.com>
Date: Mon, 29 Dec 2025 16:40:46 +0530
Message-ID: 
 <CALDaNm2T__Uha1fn274bP3jKDSwm37ewWYvA+vOo75FTT2t3SA@mail.gmail.com>
Subject: Re: [Proposal] Adding Log File Capability to pg_createsubscriber
To: Gyan Sreejith <gyan.sreejith@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Euler Taveira <euler@eulerto.com>,
	"kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
 Peter Smith <smithpb2250@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CALDaNm2T__Uha1fn274bP3jKDSwm37ewWYvA%2BvOo75FTT2t3SA%40mail.gmail.com>
Precedence: bulk

On Wed, 24 Dec 2025 at 04:52, Gyan Sreejith <gyan.sreejith@gmail.com> wrote=
:
>
> Thank you for the feedback everybody. As I read through this email chain,=
 I found differing opinions on how logging should be implemented. This ambi=
guity leaves me unsure as to which solution(s) to pursue. As of right now, =
I have attached the git-format patch like Hayato Kuroda recommended (but it=
 does not have any new changes). I am willing to implement whatever solutio=
n when we reach a consensus.

Few comments:
1) The file permissions are 664 for pg_createsubscriber_internal.log,
pg_createsubscriber_resetwal.log but 600 for
pg_createsubscriber_server.log. The permissions should be the same for
all the files.
...
if (opt->log_dir !=3D NULL)
out_file =3D psprintf("%s/pg_createsubscriber_resetwal.log", opt->log_dir);
else
out_file =3D DEVNULL;

cmd_str =3D psprintf("\"%s\" -D \"%s\" > \"%s\"", pg_resetwal_path,
   subscriber_dir, out_file);

pg_log_debug("pg_resetwal command is: %s", cmd_str);
...

...
if (opt->log_dir !=3D NULL)
{
appendPQExpBuffer(pg_ctl_cmd, " -l %s/pg_createsubscriber_server.log",
opt->log_dir);
}

pg_log_debug("pg_ctl command is: %s", pg_ctl_cmd->data);
rc =3D system(pg_ctl_cmd->data);
...
2)  Can you gracefully handle the case where permissions are not
enough in the directory and throw proper error:
if (stat(opt.log_dir, &statbuf) !=3D 0)
{
if (errno =3D=3D ENOENT)
{
mkdir(opt.log_dir, S_IRWXU);
pg_log_info("log directory created");
}
else
pg_fatal("could not access directory \"%s\": %m", opt.log_dir);
}

3) Currently there is no timestamp included for
pg_createsubscriber_internal and pg_createsubscriber_resetwal log file
contents. Without that it is difficult to tell when the operations
were done. It will be good to include them.

4) The patch does not apply on the head, kindly rebase on top of head.

5) Do you need to open and close the log file each time?
...
if (internal_log_file !=3D NULL)
{
if ((fp =3D fopen(internal_log_file, "a")) =3D=3D NULL)
pg_fatal("could not write to log file \"%s\": %m", internal_log_file);

fprintf(fp, "checking settings on subscriber\n");
fclose(fp);
}
else
pg_log_info("checking settings on subscriber");
...

...
if (internal_log_file !=3D NULL)
{
if ((fp =3D fopen(internal_log_file, "a")) =3D=3D NULL)
pg_fatal("could not write to log file \"%s\": %m", internal_log_file);

fprintf(fp, "checking settings on publisher\n");
fclose(fp);
}
else
pg_log_info("checking settings on publisher");
...

Regards,
Vignesh