public inbox for [email protected]
help / color / mirror / Atom feedFrom: Gyan Sreejith <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: vignesh C <[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: Tue, 23 Dec 2025 18:22:11 -0500
Message-ID: <CAEqnbaVVp_g1m1nhOBZjtiAz5W-xwPQCmWH4hjYYehA+ktYg9Q@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1+2PYU-y2weY_zFeysP8Ux45iBNdaYMZuJtMTqXwCxawQ@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>
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.
Thank you for all of the help,
Gyan Sreejith
On Thu, Dec 18, 2025 at 1:49 AM Amit Kapila <[email protected]> wrote:
> On Thu, Dec 18, 2025 at 6:59 AM Euler Taveira <[email protected]> wrote:
> >
> > On Wed, Dec 17, 2025, at 7:07 AM, vignesh C wrote:
> > >
> > > By providing this as an option, users can store the log files outside
> > > the data directory, eliminating the need for any additional handling
> > > during backups.
> > >
> >
> > Do we really need an option to capture the stdout / stderr output to a
> file? I
> > doubt it. There is already various ways to capture. psql and pg_upgrade
> are the
> > only tools that have this option.
> >
>
> pg_ctl also has the -l option. I think any place where long
> text/errors can be outputted, a log file is preferred because one
> could later parse it to know the exact details. Also, splitting the
> log as proposed here or in pg_upgrade helps to navigate the LOG like
> is the problem in start/stop of the server or a pub-sub setup?
> Similarly the log can be splitted for pub/sub specific information.
> There appears to be some useful information like:
>
> pg_createsubscriber: warning: two_phase option will not be enabled for
> replication slots
> pg_createsubscriber: detail: Subscriptions will be created with the
> two_phase option disabled. Prepared transactions will be replicated at
> COMMIT PREPARED.
> pg_createsubscriber: hint: You can use the command-line option
> --enable-two-phase to enable two_phase.
>
> I think it will be useful to LOG this separately from the main LOG [1]
> (which can contain server specific info as follows) so that users can
> consider running pg_createsubscriber with additional options or
> changing the subscriber configuration once setup is complete.
>
> [1]:
> [startup] LOG: database system was interrupted; last known up at
> 2025-12-17 14:46:07 IST
> [startup] LOG: starting backup recovery with redo LSN 0/06000028,
> checkpoint LSN 0/06000080, on timeline ID 1
> [startup] LOG: entering standby mode
> [startup] LOG: redo starts at 0/06000028
> [startup] LOG: completed backup recovery with redo LSN 0/06000028 and
> end LSN 0/06000120
> [startup] LOG: consistent recovery state reached at 0/06000120
>
> --
> With Regards,
> Amit Kapila.
>
Attachments:
[application/octet-stream] 0001-Add-a-new-argument-l-logdir-to-pg_createsubscriber.patch (9.5K, 3-0001-Add-a-new-argument-l-logdir-to-pg_createsubscriber.patch)
download | inline diff:
From cf182c65d2add37042f88515508e96439982ee44 Mon Sep 17 00:00:00 2001
From: Gyan Sreejith <[email protected]>
Date: Tue, 23 Dec 2025 18:07:10 -0500
Subject: [PATCH] Add a new argument -l <logdir> to pg_createsubscriber.
Enabling the option will create three new log files -
1. logdir/pg_createsubscriber_server.log - captures messages related to starting and stopping the standby server.
2. logdir/pg_createsubscriber_resetwal.log - captures the output of pg_resetwal.
3. logdir/pg_createsubscriber_internal.log - captures internal diagnostic output from pg_createsubscriber.
---
doc/src/sgml/ref/pg_createsubscriber.sgml | 27 ++++++++
src/bin/pg_basebackup/pg_createsubscriber.c | 63 +++++++++++++++++--
.../t/040_pg_createsubscriber.pl | 36 ++++++++++-
3 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index bb9cc72576c..5e24a297566 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -136,6 +136,33 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-l <replaceable class="parameter">directory</replaceable></option></term>
+ <term><option>--logdir=<replaceable class="parameter">directory</replaceable></option></term>
+ <listitem>
+ <para>
+ The target directory in which the following three log files will be created:
+ <itemizedlist>
+ <listitem>
+ <para>
+ pg_createsubscriber_server.log which captures logs related to stopping and starting the standby server,
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ pg_createsubscriber_resetwal.log which captures the output of pg_resetwal, and
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ pg_createsubscriber_internal.log which captures internal diagnostic output (validations, checks, etc.)
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-n</option></term>
<term><option>--dry-run</option></term>
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index f59c293d875..e8d7b0730e9 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -36,6 +36,7 @@
struct CreateSubscriberOptions
{
char *config_file; /* configuration file */
+ char *log_dir; /* log directory */
char *pub_conninfo_str; /* publisher connection string */
char *socket_dir; /* directory for Unix-domain socket, if any */
char *sub_port; /* subscriber port number */
@@ -150,6 +151,8 @@ static pg_prng_state prng_state;
static char *pg_ctl_path = NULL;
static char *pg_resetwal_path = NULL;
+static char *internal_log_file = NULL;
+
/* standby / subscriber data directory */
static char *subscriber_dir = NULL;
@@ -251,6 +254,7 @@ usage(void)
" databases and databases that don't allow connections\n"));
printf(_(" -d, --database=DBNAME database in which to create a subscription\n"));
printf(_(" -D, --pgdata=DATADIR location for the subscriber data directory\n"));
+ printf(_(" -l, --logdir=LOGDIR location for the new log directory\n"));
printf(_(" -n, --dry-run dry run, just show what would be done\n"));
printf(_(" -p, --subscriber-port=PORT subscriber port number (default %s)\n"), DEFAULT_SUB_PORT);
printf(_(" -P, --publisher-server=CONNSTR publisher connection string\n"));
@@ -670,6 +674,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
bool crc_ok;
struct timeval tv;
+ char *out_file;
char *cmd_str;
pg_log_info("modifying system identifier of subscriber");
@@ -703,8 +708,14 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
else
pg_log_info("running pg_resetwal on the subscriber");
+
+ 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, DEVNULL);
+ subscriber_dir, out_file);
pg_log_debug("pg_resetwal command is: %s", cmd_str);
@@ -893,8 +904,18 @@ check_publisher(const struct LogicalRepInfo *dbinfo)
int cur_walsenders;
int max_prepared_transactions;
char *max_slot_wal_keep_size;
+ FILE *fp;
+
+ 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);
- pg_log_info("checking settings on publisher");
+ fprintf(fp, "checking settings on publisher\n");
+ fclose(fp);
+ }
+ else
+ pg_log_info("checking settings on publisher");
conn = connect_database(dbinfo[0].pubconninfo, true);
@@ -1028,8 +1049,18 @@ check_subscriber(const struct LogicalRepInfo *dbinfo)
int max_lrworkers;
int max_reporigins;
int max_wprocs;
+ FILE *fp;
+
+ 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);
- pg_log_info("checking settings on subscriber");
+ fprintf(fp, "checking settings on subscriber\n");
+ fclose(fp);
+ }
+ else
+ pg_log_info("checking settings on subscriber");
conn = connect_database(dbinfo[0].subconninfo, true);
@@ -1548,6 +1579,11 @@ start_standby_server(const struct CreateSubscriberOptions *opt, bool restricted_
if (restrict_logical_worker)
appendPQExpBufferStr(pg_ctl_cmd, " -o \"-c max_logical_replication_workers=0\"");
+ 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);
pg_ctl_status(pg_ctl_cmd->data, rc);
@@ -2071,6 +2107,7 @@ main(int argc, char **argv)
{"all", no_argument, NULL, 'a'},
{"database", required_argument, NULL, 'd'},
{"pgdata", required_argument, NULL, 'D'},
+ {"logdir", required_argument, NULL, 'l'},
{"dry-run", no_argument, NULL, 'n'},
{"subscriber-port", required_argument, NULL, 'p'},
{"publisher-server", required_argument, NULL, 'P'},
@@ -2129,6 +2166,7 @@ main(int argc, char **argv)
/* Default settings */
subscriber_dir = NULL;
opt.config_file = NULL;
+ opt.log_dir = NULL;
opt.pub_conninfo_str = NULL;
opt.socket_dir = NULL;
opt.sub_port = DEFAULT_SUB_PORT;
@@ -2157,7 +2195,7 @@ main(int argc, char **argv)
get_restricted_token();
- while ((c = getopt_long(argc, argv, "ad:D:np:P:s:t:TU:v",
+ while ((c = getopt_long(argc, argv, "ad:D:l:np:P:s:t:TU:v",
long_options, &option_index)) != -1)
{
switch (c)
@@ -2178,6 +2216,23 @@ main(int argc, char **argv)
subscriber_dir = pg_strdup(optarg);
canonicalize_path(subscriber_dir);
break;
+ case 'l':
+ opt.log_dir = pg_strdup(optarg);
+ canonicalize_path(opt.log_dir);
+
+ 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);
+ }
+
+ internal_log_file = psprintf("%s/pg_createsubscriber_internal.log", opt.log_dir);
+ break;
case 'n':
dry_run = true;
break;
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 3d6086dc489..1712adc0439 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -13,7 +13,8 @@ program_help_ok('pg_createsubscriber');
program_version_ok('pg_createsubscriber');
program_options_handling_ok('pg_createsubscriber');
-my $datadir = PostgreSQL::Test::Utils::tempdir;
+my $datadir = PostgreSQL::Test::Utils::tempdir + "/datadir";
+my $logdir = PostgreSQL::Test::Utils::tempdir + "/logdir";
# Generate a database with a name made of a range of ASCII characters.
# Extracted from 002_pg_upgrade.pl.
@@ -537,10 +538,43 @@ my $sysid_s = $node_s->safe_psql('postgres',
'SELECT system_identifier FROM pg_control_system()');
isnt($sysid_p, $sysid_s, 'system identifier was changed');
+$node_p->backup('backup_3');
+
+# Set up node R as a logical replica node
+my $node_r = PostgreSQL::Test::Cluster->new('node_r');
+$node_r->init_from_backup($node_p, 'backup_3', has_streaming => 1);
+$node_r->append_conf(
+ 'postgresql.conf', qq[
+primary_conninfo = '$pconnstr dbname=postgres'
+hot_standby_feedback = on
+]);
+$node_r->set_standby_mode();
+
+# Test that --logdir works for pg_createsubscriber
+command_ok(
+ [
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $node_r->data_dir,
+ '--publisher-server' => $pconnstr,
+ '--database' => 'postgres',
+ '--logdir' => $logdir,
+ ],
+ 'check for log file creation for pg_createSubscriber');
+
+# Check that all log files were created
+ok( -f "$logdir/pg_createsubscriber_server.log",
+ 'pg_createsubscriber_server.log file was created');
+ok(-f "$logdir/pg_createsubscriber_resetwal.log",
+ 'pg_resetwal.log file was created');
+ok( -f "$logdir/pg_createsubscriber_internal.log",
+ 'pg_createsubsriber.log file was created');
+
# clean up
$node_p->teardown_node;
$node_s->teardown_node;
$node_t->teardown_node;
$node_f->teardown_node;
+$node_r->teardown_node;
done_testing();
--
2.43.0
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: <CAEqnbaVVp_g1m1nhOBZjtiAz5W-xwPQCmWH4hjYYehA+ktYg9Q@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