From 98f4e323cc84003990bddb403134907a27eb96fe Mon Sep 17 00:00:00 2001 From: Gyan Sreejith Date: Tue, 24 Mar 2026 20:02:58 -0400 Subject: [PATCH v19] Add a new argument -l 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 | 192 +++++++++++++++++- .../t/040_pg_createsubscriber.pl | 48 ++++- 3 files changed, 254 insertions(+), 15 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 + + + + + + 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 pg_createsubscriber + 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. + + + + pg_createsubscriber_server.log which captures logs + related to stopping and starting the standby server, + + + + + pg_createsubscriber_internal.log which captures + internal diagnostic output (validations, checks, etc.) + + + + + + + diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index b2bc9dae0b8..d795e2cb47e 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.log" +#define INTERNAL_LOG_FILE_NAME "pg_createsubscriber_internal.log" + /* 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,15 @@ 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, + enum pg_log_part part, + const char *pg_restrict fmt, va_list args) + pg_attribute_printf(3, 0); #define WAIT_INTERVAL 1 /* 1 second */ @@ -172,6 +184,11 @@ 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]; /* Subdirectory of the user specified logdir + * where the log files are written (if + * specified) */ + /* standby / subscriber data directory */ static char *subscriber_dir = NULL; @@ -180,8 +197,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. + * Writes to stderr, and also to the log file (if pg_createsubscriber --logdir option was 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, part, 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 +228,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 +243,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); @@ -327,6 +365,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 +800,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 +839,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", 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 +869,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt) } pg_free(cf); + pg_free(cmd_str); } /* @@ -1023,6 +1076,99 @@ server_is_in_recovery(PGconn *conn) return ret == 0; } +static void +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); + + /* Do nothing if log level is too low. */ + 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"); + fflush(internal_log_file_fp); +} + +/* + * Open a new logfile with proper permissions. + */ +static FILE * +logfile_open(const char *filename, const char *mode) +{ + FILE *fh; + + fh = fopen(filename, mode); + + if (!fh) + 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 (basedir/timestamp) is too long"); + + /* 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 +1927,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\"", 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 +2500,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 +2559,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 +2590,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 +2611,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 +2762,28 @@ 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", 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" @@ -2713,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; /* 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.43.0