From: Amit Kapila Date: Wed, 25 Mar 2026 05:52:07 +0000 (+0530) Subject: pg_createsubscriber: Add -l/--logdir option to redirect output to files. X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6b5b7eae3ae65572e2abda20ef2b2c908527c28a;p=thirdparty%2Fpostgresql.git pg_createsubscriber: Add -l/--logdir option to redirect output to files. This commit introduces a -l (or --logdir) argument to pg_createsubscriber, allowing users to specify a directory for log files. When enabled, a timestamped subdirectory is created within the specified log directory, containing: pg_createsubscriber_server.log: Captures logs from the standby server during its start/stop cycles. pg_createsubscriber_internal.log: Captures the tool's own internal diagnostic and progress messages. This ensures that transient server and utility messages are preserved for troubleshooting after the subscriber creation process completes or errored out. Author: Gyan Sreejith Author: Hayato Kuroda Reviewed-by: vignesh C Reviewed-by: Euler Taveira Reviewed-by: shveta malik Reviewed-by: Amit Kapila Reviewed-by: Shlok Kyal Reviewed-by: Peter Smith Reviewed-by: Chao Li Discussion: https://postgr.es/m/CAEqnbaUthOQARV1dscGvB_EsqC-YfxiM6rWkVDHc+G+f4oSUHw@mail.gmail.com --- diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml index 6e17cee18eb..3b3038d1891 100644 --- a/doc/src/sgml/ref/pg_createsubscriber.sgml +++ b/doc/src/sgml/ref/pg_createsubscriber.sgml @@ -136,6 +136,40 @@ 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. + + + + 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.) + + + + By default, the umask is set to 077 so that the + log files are only readable by the user running the command. However, if + the target data directory is configured to allow group-read access, + pg_createsubscriber will adjust the log file + permissions to match. This ensures that the log file security remains + consistent with the database cluster itself. + + + + diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index b2bc9dae0b8..44c10a71d29 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,31 @@ 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 --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 +230,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 +245,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 +367,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 +802,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 +841,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 +871,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt) } pg_free(cf); + pg_free(cmd_str); } /* @@ -1023,6 +1078,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; + + /* Add 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 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 +1929,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 +2502,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 +2561,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 +2592,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 +2613,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 +2764,31 @@ main(int argc, char **argv) exit(1); } + if (opt.log_dir != NULL) + { + char *internal_log_file; + + umask(PG_MODE_MASK_OWNER); + + /* + * Set mask based on PGDATA permissions, needed for the creation of + * the output directories with correct permissions, similar with + * pg_ctl and pg_upgrade. + * + * Don't error here if the data directory cannot be stat'd. Upcoming + * checks for the data directory would raise the fatal error later. + */ + if (GetDataDirectoryCreatePerm(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" diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 0c27fca7bb7..858082c70df 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -14,6 +14,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 +363,35 @@ 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"); + # 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 +471,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 +568,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 +582,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 +610,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");