public inbox for [email protected]
help / color / mirror / Atom feedFrom: vignesh C <[email protected]>
To: Gyan Sreejith <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Subject: Re: [Proposal] Adding Log File Capability to pg_createsubscriber
Date: Mon, 29 Dec 2025 16:40:46 +0530
Message-ID: <CALDaNm2T__Uha1fn274bP3jKDSwm37ewWYvA+vOo75FTT2t3SA@mail.gmail.com> (raw)
In-Reply-To: <CAEqnbaVVp_g1m1nhOBZjtiAz5W-xwPQCmWH4hjYYehA+ktYg9Q@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>
On Wed, 24 Dec 2025 at 04:52, Gyan Sreejith <[email protected]> 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 ambiguity 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 solution 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 != NULL)
out_file = psprintf("%s/pg_createsubscriber_resetwal.log", opt->log_dir);
else
out_file = DEVNULL;
cmd_str = 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 != 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 = 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) != 0)
{
if (errno == 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 != NULL)
{
if ((fp = fopen(internal_log_file, "a")) == 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 != NULL)
{
if ((fp = fopen(internal_log_file, "a")) == 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
view thread (55+ 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]
Subject: Re: [Proposal] Adding Log File Capability to pg_createsubscriber
In-Reply-To: <CALDaNm2T__Uha1fn274bP3jKDSwm37ewWYvA+vOo75FTT2t3SA@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