I was able to make the first two changes that you suggested. However, I was not able to get the test to run with one of the existing replica servers. I have attached the patch for the first two changes. Thank you!
Regards, Gyan On Wed, Feb 4, 2026 at 4:44 AM vignesh C <[email protected]> wrote: > On Mon, 2 Feb 2026 at 04:35, Gyan Sreejith <[email protected]> > wrote: > > > > Thank you! > > > > I have made the suggested changes. In addition, I added a wrapper for > pg_fatal to write to the file and then do everything that pg_fatal would do. > > I have attached the patch. > > Few comments: > 1) Can we change the macro names: > INFO -> pg_log_info > INFO_HINT -> pg_log_info_hint > DEBUG -> pg_log_debug > FATAL -> pg_fatal > > if required do an #undef and call pg_log_generic in the else similar > to how it is done in pg_backup_utils.h > > +#define INFO(...) do{\ > + if (internal_log_file_fp != NULL) \ > + internal_log_file_write(__VA_ARGS__); \ > + else \ > + pg_log_info(__VA_ARGS__);\ > +} while(0) > + > +#define INFO_HINT(...) do{\ > + if (internal_log_file_fp != NULL) \ > + internal_log_file_write(__VA_ARGS__); \ > + else \ > + pg_log_info_hint(__VA_ARGS__);\ > +} while(0) > + > +#define DEBUG(...) do{\ > + if (internal_log_file_fp != NULL) \ > + internal_log_file_write(__VA_ARGS__); \ > + else \ > + pg_log_debug(__VA_ARGS__);\ > +} while(0) > + > +#define FATAL(...) do{\ > + if (internal_log_file_fp != NULL) \ > + internal_log_file_write(__VA_ARGS__); \ > + pg_fatal(__VA_ARGS__); /* call pg_fatal after writing to logs */ \ > +} while(0) > > 2) You can keep the variabled in case 'l' handling to keep the scope > accordingly: > + char timestamp[128]; > + struct timeval tval; > + time_t now; > > 3) Instead of creating a new standby and running with -l option, can > you run it with -l for one of the existing tests: > +$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 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 @internal_log_files = glob > "$logdir/pg_createsubscriber_internal_*.log"; > +is( scalar(@internal_log_files), 1, " > + pg_createsubscriber_internal.log file was created"); > > Regards, > Vignesh >
From 8f3237b07d72eb4443e025c381ff5ae7221a4141 Mon Sep 17 00:00:00 2001 From: Gyan Sreejith <[email protected]> Date: Sun, 8 Feb 2026 20:57:07 -0500 Subject: [PATCH v5] 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, and two new logfiles (with timestamps in their names) are added to the directory: 1. pg_createsubscriber_server_timestamp.log - captures messages related to starting and stopping the standby server. 2. pg_createsubscriber_internal_timestamp.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 2026-01-19-20-43-17.204240, a directory abc is created if it doesn't exist already, and the directory will contain the two log files pg_createsubscriber_server_2026-01-19-20-43-17.204240.log and pg_createsubscriber_internal_2026-01-19-20-43-17.204240.log --- doc/src/sgml/ref/pg_createsubscriber.sgml | 22 +++ src/bin/pg_basebackup/pg_createsubscriber.c | 162 +++++++++++++++++- .../t/040_pg_createsubscriber.pl | 36 +++- 3 files changed, 215 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml index cf45ff3573d..8cb70403931 100644 --- a/doc/src/sgml/ref/pg_createsubscriber.sgml +++ b/doc/src/sgml/ref/pg_createsubscriber.sgml @@ -136,6 +136,28 @@ 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. The following two log files will be created with a umask of 077 so that access is disallowed to other users by default. The timestamp that is added to the name will indicate the time at which pg_createsubscriber was run. + <itemizedlist> + <listitem> + <para> + pg_createsubscriber_server_timestamp.log which captures logs related to stopping and starting the standby server, + </para> + </listitem> + <listitem> + <para> + pg_createsubscriber_internal_timestamp.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 2bc84505aab..598cb07a1bf 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -49,10 +49,48 @@ #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" + +#undef pg_log_info +#define pg_log_info(...) do{\ + if (internal_log_file_fp != NULL) \ + internal_log_file_write(__VA_ARGS__); \ + else \ + pg_log_generic(PG_LOG_INFO,PG_LOG_PRIMARY,__VA_ARGS__);\ +} while(0) + +#undef pg_log_info_hint +#define pg_log_info_hint(...) do{\ + if (internal_log_file_fp != NULL) \ + internal_log_file_write(__VA_ARGS__); \ + else \ + pg_log_generic(PG_LOG_INFO, PG_LOG_HINT, __VA_ARGS__);\ +} while(0) + +#undef pg_log_debug +#define pg_log_debug(...) do{\ + if (internal_log_file_fp != NULL) \ + internal_log_file_write(__VA_ARGS__); \ + else \ + if (unlikely(__pg_log_level <= PG_LOG_DEBUG)) \ + pg_log_generic(PG_LOG_DEBUG, PG_LOG_PRIMARY, __VA_ARGS__); \ +} while(0) + +#undef pg_fatal +#define pg_fatal(...) do{\ + if (internal_log_file_fp != NULL) \ + internal_log_file_write(__VA_ARGS__); \ + pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \ + exit(1); \ +} while(0) + + /* 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 */ @@ -146,6 +184,9 @@ static void drop_existing_subscription(PGconn *conn, const char *subname, const char *dbname); static void get_publisher_databases(struct CreateSubscriberOptions *opt, bool dbnamespecified); +static void + internal_log_file_write(const char *format,...) __attribute__((format(printf, 1, 2))); + #define WAIT_INTERVAL 1 /* 1 second */ @@ -167,6 +208,10 @@ 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 *log_timestamp = NULL; /* Timestamp to be used in all log file + * names */ + /* standby / subscriber data directory */ static char *subscriber_dir = NULL; @@ -174,6 +219,50 @@ static bool recovery_ended = false; static bool standby_running = false; static bool recovery_params_set = false; +static void +internal_log_file_write(const char *format,...) +{ + if (internal_log_file_fp != NULL) + { + va_list args; + + va_start(args, format); + vfprintf(internal_log_file_fp, format, args); + fprintf(internal_log_file_fp, "\n"); + fflush(internal_log_file_fp); + va_end(args); + } +} + +/* + * 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 + pg_fatal("could not open log file \"%s\": %m", + filename); + + return fh; +} /* * Clean up objects created by pg_createsubscriber. @@ -269,6 +358,12 @@ cleanup_objects_atexit(void) if (standby_running) stop_standby_server(subscriber_dir); + + if (internal_log_file_fp != NULL) + { + fclose(internal_log_file_fp); + internal_log_file_fp = NULL; + } } static void @@ -283,6 +378,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")); @@ -702,6 +798,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"); @@ -713,7 +810,7 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt) /* * Select a new system identifier. * - * XXX this code was extracted from BootStrapXLOG(). + * XXX this code was extracted from BootStrapXpg_log_info(). */ gettimeofday(&tv, NULL); cf->system_identifier = ((uint64) tv.tv_sec) << 32; @@ -735,8 +832,14 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt) else pg_log_info("running pg_resetwal on the subscriber"); - cmd_str = psprintf("\"%s\" -D \"%s\" > \"%s\"", pg_resetwal_path, - subscriber_dir, DEVNULL); + + if (opt->log_dir != NULL) + out_file = psprintf("%s/%s_%s.log", opt->log_dir, SERVER_LOG_FILE_NAME, log_timestamp); + 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); @@ -1650,6 +1753,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/%s_%s.log", opt->log_dir, SERVER_LOG_FILE_NAME, log_timestamp); + } + 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); @@ -2181,6 +2289,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'}, @@ -2215,6 +2324,7 @@ main(int argc, char **argv) char *consistent_lsn; char pidfile[MAXPGPATH]; + char *internal_log_file; pg_logging_init(argv[0]); pg_logging_set_level(PG_LOG_WARNING); @@ -2239,6 +2349,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; @@ -2267,7 +2378,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) @@ -2288,6 +2399,46 @@ main(int argc, char **argv) subscriber_dir = pg_strdup(optarg); canonicalize_path(subscriber_dir); break; + case 'l': + { + char timestamp[128]; + struct timeval tval; + time_t now; + struct tm tmbuf; + + gettimeofday(&tval, NULL); + now = tval.tv_sec; + strftime(timestamp, sizeof(timestamp), "%Y-%m-%d-%H-%M-%S", localtime_r(&now, &tmbuf)); + /* append microseconds */ + snprintf(timestamp + strlen(timestamp), sizeof(timestamp) - strlen(timestamp), + ".%06u", (unsigned int) (tval.tv_usec)); + log_timestamp = pg_strdup(timestamp); + + opt.log_dir = pg_strdup(optarg); + canonicalize_path(opt.log_dir); + + if (stat(opt.log_dir, &statbuf) != 0) + { + if (errno == ENOENT) + { + if (mkdir(opt.log_dir, S_IRWXU) == 0) + pg_log_info("log directory created"); + else if (errno == EACCES) + pg_fatal("permission denied trying to create log directory \"%s\": %m", opt.log_dir); + else + pg_fatal("could not create log directory \"%s\": %m", opt.log_dir); + } + else if (errno == EACCES) + pg_fatal("permission denied trying to access directory \"%s\": %m", opt.log_dir); + else + pg_fatal("could not access directory \"%s\": %m", opt.log_dir); + } + internal_log_file = psprintf("%s/%s_%s.log", opt.log_dir, INTERNAL_LOG_FILE_NAME, log_timestamp); + if ((internal_log_file_fp = logfile_open(internal_log_file, "a")) == NULL) + pg_fatal("could not open log file \"%s\": %m", internal_log_file); + + break; + } case 'n': dry_run = true; break; @@ -2621,5 +2772,8 @@ main(int argc, char **argv) pg_log_info("Done!"); + if (internal_log_file_fp != NULL) + fclose(internal_log_file_fp); + return 0; } diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 0c27fca7bb7..cdb12623b3b 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. @@ -629,10 +630,43 @@ command_ok( # not being tracked, something that is set within $node->start(). system_log('pg_ctl', 'stop', '--pgdata', $node_k->data_dir); +$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 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 @internal_log_files = glob "$logdir/pg_createsubscriber_internal_*.log"; +is( scalar(@internal_log_files), 1, " + pg_createsubscriber_internal.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
