public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Gyan Sreejith' <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: shveta malik <[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: Tue, 24 Mar 2026 10:06:08 +0000
Message-ID: <OS9PR01MB1214926DB01BF19C09368188EF548A@OS9PR01MB12149.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAEqnbaULSSQcir8Z8KDXduV4o4NZvJLeBMZN0VW3wh5QZjpEnw@mail.gmail.com>
References: <OSOPR01MB1215385EAC6A76650444E40B0F54FA@OSOPR01MB12153.jpnprd01.prod.outlook.com>
<CAEqnbaU=EgqhxEx0ig4TdY8pdt0Vn+vmCJBuoORRHOzovW9dWA@mail.gmail.com>
<TYRPR01MB12156F431E4B83ECFF14034BDF54CA@TYRPR01MB12156.jpnprd01.prod.outlook.com>
<CAEqnbaXb-TKUm7P-=_zrgQ=shRXkkZscPOHEL9OS6Cb2V8YT8Q@mail.gmail.com>
<CAA4eK1JiAzaY7UYnya7uDhX-kAL1PrsOAxBtys7c35t4q4H3_A@mail.gmail.com>
<CAEqnbaVF0yXQk=VVzr-8V23E=iUpUqyah6hxx2fy0ZVJh+CaGA@mail.gmail.com>
<CAJpy0uAwPNG3GN5+jTajnccLe+F1mW6dZh-9112u3QQQYugA=Q@mail.gmail.com>
<CAA4eK1JyYHPgaPTow5co8wBh8Ga+fx=vqHLhk2zHeqdOGYN4Vg@mail.gmail.com>
<OS9PR01MB12149FECFFF25973C167F9CFAF54BA@OS9PR01MB12149.jpnprd01.prod.outlook.com>
<CAEqnbaULSSQcir8Z8KDXduV4o4NZvJLeBMZN0VW3wh5QZjpEnw@mail.gmail.com>
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.gma...
[2]: https://www.postgresql.org/message-id/OS9PR01MB12149F1AD9C79A2644753A18AF548A%40OS9PR01MB12149.jpnpr...
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v18-0001-Add-a-new-argument-l-logdir-to-pg_createsubscrib.patch (16.5K, 2-v18-0001-Add-a-new-argument-l-logdir-to-pg_createsubscrib.patch)
download | inline diff:
From 47a1a9d5d2f652813cc9f10bb74026ff19d3d17d Mon Sep 17 00:00:00 2001
From: Gyan Sreejith <[email protected]>
Date: Mon, 23 Mar 2026 21:21:56 -0400
Subject: [PATCH v18 1/2] Add a new argument -l <logdir> to
pg_createsubscriber.
Enabling the option to write messages to log files in the specified directory.
A new directory is created if required. A subdirectory is created with timestamp as its name, and it will contain two new logfiles:
1. pg_createsubscriber_server.log - captures messages related to starting and stopping the standby server.
2. pg_createsubscriber_internal.log - captures internal diagnostic output from pg_createsubscriber.
For example, if we specify -l abc as an argument, and if the timestamp on running it is 20260119T204317.204, a directory abc is created if it doesn't exist already, with 20260119T204317.204 as its subdirectory and it will contain the two log files pg_createsubscriber_server.log and pg_createsubscriber_internal.log
---
doc/src/sgml/ref/pg_createsubscriber.sgml | 29 +++
src/bin/pg_basebackup/pg_createsubscriber.c | 174 +++++++++++++++++-
.../t/040_pg_createsubscriber.pl | 48 ++++-
3 files changed, 239 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index 6e17cee18eb..5ac61c7b558 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -136,6 +136,35 @@ 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>
+ Specify the name of the log directory. A new directory is created with
+ this name if it does not exist. A subdirectory with a timestamp
+ indicating the time at which <application>pg_createsubscriber</application>
+ was run will be created. The following two log files will be created in
+ the subdirectory with a umask of 077 so that access is disallowed to
+ other users by default.
+ <itemizedlist>
+ <listitem>
+ <para>
+ <filename>pg_createsubscriber_server.log</filename> which captures logs
+ related to stopping and starting the standby server,
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <filename>pg_createsubscriber_internal.log</filename> 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 b2bc9dae0b8..63ffd337613 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -20,6 +20,7 @@
#include "common/connect.h"
#include "common/controldata_utils.h"
+#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/logging.h"
#include "common/pg_prng.h"
@@ -49,10 +50,14 @@
#define INCLUDED_CONF_FILE "pg_createsubscriber.conf"
#define INCLUDED_CONF_FILE_DISABLED INCLUDED_CONF_FILE ".disabled"
+#define SERVER_LOG_FILE_NAME "pg_createsubscriber_server"
+#define INTERNAL_LOG_FILE_NAME "pg_createsubscriber_internal"
+
/* Command-line options */
struct CreateSubscriberOptions
{
char *config_file; /* configuration file */
+ char *log_dir; /* log directory name */
char *pub_conninfo_str; /* publisher connection string */
char *socket_dir; /* directory for Unix-domain socket, if any */
char *sub_port; /* subscriber port number */
@@ -149,8 +154,14 @@ static void get_publisher_databases(struct CreateSubscriberOptions *opt,
static void report_createsub_log(enum pg_log_level, enum pg_log_part,
const char *pg_restrict fmt,...)
pg_attribute_printf(3, 4);
+static void report_createsub_log_v(enum pg_log_level level, enum pg_log_part part,
+ const char *pg_restrict fmt, va_list args)
+ pg_attribute_printf(3, 0);
pg_noreturn static void report_createsub_fatal(const char *pg_restrict fmt,...)
pg_attribute_printf(1, 2);
+static void internal_log_file_write(enum pg_log_level level,
+ const char *pg_restrict fmt, va_list args)
+ pg_attribute_printf(2, 0);
#define WAIT_INTERVAL 1 /* 1 second */
@@ -172,6 +183,9 @@ static pg_prng_state prng_state;
static char *pg_ctl_path = NULL;
static char *pg_resetwal_path = NULL;
+static FILE *internal_log_file_fp = NULL; /* File ptr to log all messages to */
+static char logdir[MAXPGPATH]; /* Directory log files are put (if specified) */
+
/* standby / subscriber data directory */
static char *subscriber_dir = NULL;
@@ -180,8 +194,29 @@ static bool standby_running = false;
static bool recovery_params_set = false;
/*
- * Report a message with a given log level
+ * Report a message with a given log level to stderr and log file
+ * (if specified).
*/
+static void
+report_createsub_log_v(enum pg_log_level level, enum pg_log_part part,
+ const char *pg_restrict fmt, va_list args)
+{
+ int save_errno = errno;
+
+ if (internal_log_file_fp != NULL)
+ {
+ /* Output to both stderr and the log file */
+ va_list arg_cpy;
+
+ va_copy(arg_cpy, args);
+ internal_log_file_write(level, fmt, arg_cpy);
+ va_end(arg_cpy);
+ /* Restore errno in case internal_log_file_write changed it */
+ errno = save_errno;
+ }
+ pg_log_generic_v(level, part, fmt, args);
+}
+
static void
report_createsub_log(enum pg_log_level level, enum pg_log_part part,
const char *pg_restrict fmt,...)
@@ -190,7 +225,7 @@ report_createsub_log(enum pg_log_level level, enum pg_log_part part,
va_start(args, fmt);
- pg_log_generic_v(level, part, fmt, args);
+ report_createsub_log_v(level, part, fmt, args);
va_end(args);
}
@@ -205,7 +240,7 @@ report_createsub_fatal(const char *pg_restrict fmt,...)
va_start(args, fmt);
- pg_log_generic_v(PG_LOG_ERROR, PG_LOG_PRIMARY, fmt, args);
+ report_createsub_log_v(PG_LOG_ERROR, PG_LOG_PRIMARY, fmt, args);
va_end(args);
@@ -313,6 +348,13 @@ cleanup_objects_atexit(void)
if (standby_running)
stop_standby_server(subscriber_dir);
+
+ if (internal_log_file_fp != NULL)
+ {
+ if (fclose(internal_log_file_fp) != 0)
+ report_createsub_fatal("could not close %s/%s.log: %m", logdir, INTERNAL_LOG_FILE_NAME);
+ internal_log_file_fp = NULL;
+ }
}
static void
@@ -327,6 +369,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 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"));
@@ -761,6 +804,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
bool crc_ok;
struct timeval tv;
+ char *out_file;
char *cmd_str;
report_createsub_log(PG_LOG_INFO, PG_LOG_PRIMARY,
@@ -799,8 +843,20 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
report_createsub_log(PG_LOG_INFO, PG_LOG_PRIMARY,
"running pg_resetwal on the subscriber");
- cmd_str = psprintf("\"%s\" -D \"%s\" > \"%s\"", pg_resetwal_path,
- subscriber_dir, DEVNULL);
+ /*
+ * Redirecting the output to the logfile if specified. Since the output
+ * would be very short, around one line, we do not provide a separate file
+ * for it; it's done as a part of the server log.
+ */
+ if (opt->log_dir)
+ out_file = psprintf("%s/%s.log", logdir, SERVER_LOG_FILE_NAME);
+ else
+ out_file = DEVNULL;
+
+ cmd_str = psprintf("\"%s\" -D \"%s\" >> \"%s\"", pg_resetwal_path,
+ subscriber_dir, out_file);
+ if (opt->log_dir)
+ pg_free(out_file);
report_createsub_log(PG_LOG_DEBUG, PG_LOG_PRIMARY,
"pg_resetwal command is: %s", cmd_str);
@@ -817,6 +873,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt)
}
pg_free(cf);
+ pg_free(cmd_str);
}
/*
@@ -1023,6 +1080,89 @@ server_is_in_recovery(PGconn *conn)
return ret == 0;
}
+static void
+internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
+ va_list args)
+{
+ Assert(internal_log_file_fp);
+
+ /* Do nothing if log level is too low. */
+ if (level < __pg_log_level)
+ return;
+
+ vfprintf(internal_log_file_fp, _(fmt), args);
+
+ fprintf(internal_log_file_fp, "\n");
+ fflush(internal_log_file_fp);
+}
+
+/*
+ * Open a new logfile with proper permissions.
+ * From src/backend/postmaster/syslogger.c
+ */
+static FILE *
+logfile_open(const char *filename, const char *mode)
+{
+ FILE *fh;
+ mode_t oumask;
+
+ oumask = umask((mode_t) ((~(S_IRUSR | S_IWUSR)) & (S_IRWXU | S_IRWXG | S_IRWXO)));
+ fh = fopen(filename, mode);
+ umask(oumask);
+
+ if (fh)
+ {
+ setvbuf(fh, NULL, PG_IOLBF, 0);
+
+#ifdef WIN32
+ /* use CRLF line endings on Windows */
+ _setmode(_fileno(fh), _O_TEXT);
+#endif
+ }
+ else
+ report_createsub_fatal("could not open log file \"%s\": %m",
+ filename);
+
+ return fh;
+}
+
+static void
+make_output_dirs(const char *log_basedir)
+{
+ char timestamp[128];
+ struct timeval tval;
+ time_t now;
+ struct tm tmbuf;
+ int len;
+
+ /* Generate timestamp */
+ gettimeofday(&tval, NULL);
+ now = tval.tv_sec;
+
+ strftime(timestamp, sizeof(timestamp), "%Y%m%dT%H%M%S",
+ localtime_r(&now, &tmbuf));
+
+ /* Append milliseconds */
+ snprintf(timestamp + strlen(timestamp),
+ sizeof(timestamp) - strlen(timestamp), ".%03u",
+ (unsigned int) (tval.tv_usec / 1000));
+
+ /* Build timestamp directory path */
+ len = snprintf(logdir, MAXPGPATH, "%s/%s", log_basedir, timestamp);
+
+ if (len >= MAXPGPATH)
+ report_createsub_fatal("directory path for log files, %s/%s, is too long",
+ logdir, timestamp);
+
+ /* Create base directory (ignore if exists) */
+ if (mkdir(log_basedir, pg_dir_create_mode) < 0 && errno != EEXIST)
+ report_createsub_fatal("could not create directory \"%s\": %m", log_basedir);
+
+ /* Create a timestamp-named subdirectory under the base directory */
+ if (mkdir(logdir, pg_dir_create_mode) < 0)
+ report_createsub_fatal("could not create directory \"%s\": %m", logdir);
+}
+
/*
* Is the primary server ready for logical replication?
*
@@ -1781,6 +1921,9 @@ 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)
+ appendPQExpBuffer(pg_ctl_cmd, " -l \"%s/%s.log\"", logdir, SERVER_LOG_FILE_NAME);
+
report_createsub_log(PG_LOG_DEBUG, PG_LOG_PRIMARY,
"pg_ctl command is: %s", pg_ctl_cmd->data);
rc = system(pg_ctl_cmd->data);
@@ -2351,6 +2494,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'},
@@ -2409,6 +2553,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;
@@ -2439,7 +2584,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)
@@ -2460,6 +2605,10 @@ 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);
+ break;
case 'n':
dry_run = true;
break;
@@ -2607,6 +2756,19 @@ main(int argc, char **argv)
exit(1);
}
+ if (opt.log_dir != NULL)
+ {
+ char *internal_log_file;
+
+ make_output_dirs(opt.log_dir);
+ internal_log_file = psprintf("%s/%s.log", logdir,
+ INTERNAL_LOG_FILE_NAME);
+
+ /* logfile_open() will exit if there is an error */
+ internal_log_file_fp = logfile_open(internal_log_file, "a");
+ pg_free(internal_log_file);
+ }
+
if (dry_run)
report_createsub_log(PG_LOG_INFO, PG_LOG_PRIMARY,
"Executing in dry-run mode.\n"
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 0c27fca7bb7..239ea58d9a0 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -5,6 +5,8 @@
use strict;
use warnings FATAL => 'all';
+use File::Basename;
+use File::stat;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
@@ -14,6 +16,7 @@ program_version_ok('pg_createsubscriber');
program_options_handling_ok('pg_createsubscriber');
my $datadir = PostgreSQL::Test::Utils::tempdir;
+my $logdir = PostgreSQL::Test::Utils::tempdir;
# Generate a database with a name made of a range of ASCII characters.
# Extracted from 002_pg_upgrade.pl.
@@ -362,9 +365,40 @@ command_ok(
'--subscription' => 'sub2',
'--database' => $db1,
'--database' => $db2,
+ '--logdir' => $logdir,
],
'run pg_createsubscriber --dry-run on node S');
+# Check that the log files were created
+my @server_log_files = glob "$logdir/*/pg_createsubscriber_server.log";
+is(scalar(@server_log_files),
+ 1, "pg_createsubscriber_server.log file was created");
+my $server_log_file_size = -s $server_log_files[0];
+isnt($server_log_file_size, 0,
+ "pg_createsubscriber_server.log file not empty");
+my $server_log = slurp_file($server_log_files[0]);
+like(
+ $server_log,
+ qr/consistent recovery state reached/,
+ "server reached consistent recovery state");
+
+my @internal_log_files = glob "$logdir/*/pg_createsubscriber_internal.log";
+is(scalar(@internal_log_files),
+ 1, "pg_createsubscriber_internal.log file was created");
+my $internal_log_file_size = -s $internal_log_files[0];
+isnt($internal_log_file_size, 0,
+ "pg_createsubscriber_internal.log file not empty");
+my $internal_log = slurp_file($internal_log_files[0]);
+like(
+ $internal_log,
+ qr/target server reached the consistent state/,
+ "log shows consistent state reached");
+my $timestamp_dir = dirname($internal_log_files[0]);
+my $timestamp_dir_stat = stat($timestamp_dir);
+my $timestamp_dir_mode = $timestamp_dir_stat->mode & 07777;
+is($timestamp_dir_mode, 0700,
+ "Directory with .log files has permissions S_IRWXU");
+
# Check if node S is still a standby
$node_s->start;
is($node_s->safe_psql('postgres', 'SELECT pg_catalog.pg_is_in_recovery()'),
@@ -444,7 +478,8 @@ is(scalar(() = $stderr =~ /would create subscription/g),
# Create a user-defined publication, and a table that is not a member of that
# publication.
-$node_p->safe_psql($db1, qq(
+$node_p->safe_psql(
+ $db1, qq(
CREATE PUBLICATION test_pub3 FOR TABLE tbl1;
CREATE TABLE not_replicated (a int);
));
@@ -540,8 +575,7 @@ second row
third row),
"logical replication works in database $db1");
$result = $node_s->safe_psql($db1, 'SELECT * FROM not_replicated');
-is($result, qq(),
- "table is not replicated in database $db1");
+is($result, qq(), "table is not replicated in database $db1");
# Check result in database $db2
$result = $node_s->safe_psql($db2, 'SELECT * FROM tbl2');
@@ -555,8 +589,10 @@ my $sysid_s = $node_s->safe_psql('postgres',
isnt($sysid_p, $sysid_s, 'system identifier was changed');
# Verify that pub2 was created in $db2
-is($node_p->safe_psql($db2, "SELECT COUNT(*) FROM pg_publication WHERE pubname = 'pub2'"),
- '1', "publication pub2 was created in $db2");
+is( $node_p->safe_psql(
+ $db2, "SELECT COUNT(*) FROM pg_publication WHERE pubname = 'pub2'"),
+ '1',
+ "publication pub2 was created in $db2");
# Get subscription and publication names
$result = $node_s->safe_psql(
@@ -581,7 +617,7 @@ $result = $node_s->safe_psql(
)
);
-is($result, qq($db1|{test_pub3}
+is( $result, qq($db1|{test_pub3}
$db2|{pub2}),
"subscriptions use the correct publications");
--
2.47.3
[application/octet-stream] v18-0002-Address-comments-from-Chao-Amit-and-Kuroda.patch (4.8K, 3-v18-0002-Address-comments-from-Chao-Amit-and-Kuroda.patch)
download | inline diff:
From 2a662b72c6acd469174250aaca6a7d60da856037 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Tue, 24 Mar 2026 15:43:41 +0900
Subject: [PATCH v18 2/2] Address comments from Chao, Amit and Kuroda
---
src/bin/pg_basebackup/pg_createsubscriber.c | 72 ++++++++++++---------
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 63ffd337613..1f6247a8cd8 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -160,8 +160,9 @@ static void report_createsub_log_v(enum pg_log_level level, enum pg_log_part par
pg_noreturn static void report_createsub_fatal(const char *pg_restrict fmt,...)
pg_attribute_printf(1, 2);
static void internal_log_file_write(enum pg_log_level level,
+ enum pg_log_part part,
const char *pg_restrict fmt, va_list args)
- pg_attribute_printf(2, 0);
+ pg_attribute_printf(3, 0);
#define WAIT_INTERVAL 1 /* 1 second */
@@ -209,7 +210,7 @@ report_createsub_log_v(enum pg_log_level level, enum pg_log_part part,
va_list arg_cpy;
va_copy(arg_cpy, args);
- internal_log_file_write(level, fmt, arg_cpy);
+ internal_log_file_write(level, part, fmt, arg_cpy);
va_end(arg_cpy);
/* Restore errno in case internal_log_file_write changed it */
errno = save_errno;
@@ -348,13 +349,6 @@ cleanup_objects_atexit(void)
if (standby_running)
stop_standby_server(subscriber_dir);
-
- if (internal_log_file_fp != NULL)
- {
- if (fclose(internal_log_file_fp) != 0)
- report_createsub_fatal("could not close %s/%s.log: %m", logdir, INTERNAL_LOG_FILE_NAME);
- internal_log_file_fp = NULL;
- }
}
static void
@@ -1081,8 +1075,8 @@ server_is_in_recovery(PGconn *conn)
}
static void
-internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
- va_list args)
+internal_log_file_write(enum pg_log_level level, enum pg_log_part part,
+ const char *pg_restrict fmt, va_list args)
{
Assert(internal_log_file_fp);
@@ -1090,6 +1084,30 @@ internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
if (level < __pg_log_level)
return;
+ /* Append prefix based on the log part and log level */
+ switch (part)
+ {
+ case PG_LOG_PRIMARY:
+ switch (level)
+ {
+ case PG_LOG_ERROR:
+ fprintf(internal_log_file_fp, _("error: "));
+ break;
+ case PG_LOG_WARNING:
+ fprintf(internal_log_file_fp, _("warning: "));
+ break;
+ default:
+ break;
+ }
+ break;
+ case PG_LOG_DETAIL:
+ fprintf(internal_log_file_fp, _("detail: "));
+ break;
+ case PG_LOG_HINT:
+ fprintf(internal_log_file_fp, _("hint: "));
+ break;
+ }
+
vfprintf(internal_log_file_fp, _(fmt), args);
fprintf(internal_log_file_fp, "\n");
@@ -1098,28 +1116,15 @@ internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt,
/*
* Open a new logfile with proper permissions.
- * From src/backend/postmaster/syslogger.c
*/
static FILE *
logfile_open(const char *filename, const char *mode)
{
FILE *fh;
- mode_t oumask;
- oumask = umask((mode_t) ((~(S_IRUSR | S_IWUSR)) & (S_IRWXU | S_IRWXG | S_IRWXO)));
fh = fopen(filename, mode);
- umask(oumask);
- if (fh)
- {
- setvbuf(fh, NULL, PG_IOLBF, 0);
-
-#ifdef WIN32
- /* use CRLF line endings on Windows */
- _setmode(_fileno(fh), _O_TEXT);
-#endif
- }
- else
+ if (!fh)
report_createsub_fatal("could not open log file \"%s\": %m",
filename);
@@ -1151,8 +1156,8 @@ make_output_dirs(const char *log_basedir)
len = snprintf(logdir, MAXPGPATH, "%s/%s", log_basedir, timestamp);
if (len >= MAXPGPATH)
- report_createsub_fatal("directory path for log files, %s/%s, is too long",
- logdir, timestamp);
+ report_createsub_fatal("directory path for log files \"%s/%s\" is too long",
+ log_basedir, timestamp);
/* Create base directory (ignore if exists) */
if (mkdir(log_basedir, pg_dir_create_mode) < 0 && errno != EEXIST)
@@ -2756,10 +2761,20 @@ main(int argc, char **argv)
exit(1);
}
+ /* Rudimentary check for a data directory */
+ check_data_directory(subscriber_dir);
+
if (opt.log_dir != NULL)
{
char *internal_log_file;
+ /* Set mask based on the PGDATA permissions */
+ if (!GetDataDirectoryCreatePerm(subscriber_dir))
+ report_createsub_fatal("could not read permissions of directory \"%s\": %m",
+ subscriber_dir);
+
+ umask(pg_mode_mask);
+
make_output_dirs(opt.log_dir);
internal_log_file = psprintf("%s/%s.log", logdir,
INTERNAL_LOG_FILE_NAME);
@@ -2875,9 +2890,6 @@ main(int argc, char **argv)
pg_ctl_path = get_exec_path(argv[0], "pg_ctl");
pg_resetwal_path = get_exec_path(argv[0], "pg_resetwal");
- /* Rudimentary check for a data directory */
- check_data_directory(subscriber_dir);
-
dbinfos.two_phase = opt.two_phase;
/*
--
2.47.3
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: <OS9PR01MB1214926DB01BF19C09368188EF548A@OS9PR01MB12149.jpnprd01.prod.outlook.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