From 1f72429ebf20958973edb84822c2fbbf603165f5 Mon Sep 17 00:00:00 2001 From: Hayato Kuroda Date: Wed, 18 Mar 2026 21:43:08 +0900 Subject: [PATCH v11 3/3] Address comments from Hayato Kuroda --- src/bin/pg_basebackup/pg_createsubscriber.c | 89 +++++++++------------ 1 file changed, 36 insertions(+), 53 deletions(-) diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index 87e4f95b22e..83b64960da8 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -186,8 +186,7 @@ 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 */ +static char logdir[MAXPGPATH]; /* Directory log files are put (if specified) */ /* standby / subscriber data directory */ static char *subscriber_dir = NULL; @@ -819,8 +818,13 @@ modify_subscriber_sysid(const struct CreateSubscriberOptions *opt) pg_createsub_log(PG_LOG_INFO, PG_LOG_PRIMARY, "running pg_resetwal on the subscriber"); - if (opt->log_dir != NULL) - out_file = psprintf("%s/%s/%s.log", opt->log_dir, log_timestamp, SERVER_LOG_FILE_NAME); + /* + * 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; @@ -1052,13 +1056,13 @@ static void internal_log_file_write(enum pg_log_level level, const char *pg_restrict fmt, va_list args) { - if (level < __pg_log_level) - return; + Assert(internal_log_file_fp); - if (internal_log_file_fp == NULL) + /* Do nothing if log level is too low. */ + if (level < __pg_log_level) return; - vfprintf(internal_log_file_fp, fmt, args); + vfprintf(internal_log_file_fp, _(fmt), args); fprintf(internal_log_file_fp, "\n"); fflush(internal_log_file_fp); @@ -1095,33 +1099,12 @@ logfile_open(const char *filename, const char *mode) } static void -make_dir(const char *dir) -{ - struct stat statbuf; - - if (stat(dir, &statbuf) == 0) - return; - - if (errno != ENOENT) - pg_fatal("could not stat directory \"%s\": %m", dir); - - if (mkdir(dir, S_IRWXU) == 0) - { - pg_log_info("directory %s created", dir); - return; - } - - pg_fatal("could not create log directory \"%s\": %m", dir); -} - -static void -make_output_dirs(const char *log_dir) +make_output_dirs(const char *log_basedir) { char timestamp[128]; struct timeval tval; time_t now; struct tm tmbuf; - char timestamp_dir[MAXPGPATH]; int len; /* Generate timestamp */ @@ -1136,20 +1119,20 @@ make_output_dirs(const char *log_dir) sizeof(timestamp) - strlen(timestamp), ".%03u", (unsigned int) (tval.tv_usec / 1000)); - log_timestamp = pg_strdup(timestamp); - - /* Create base directory (ignore if exists) */ - make_dir(log_dir); - /* Build timestamp directory path */ - len = snprintf(timestamp_dir, MAXPGPATH, "%s/%s", log_dir, timestamp); + len = snprintf(logdir, MAXPGPATH, "%s/%s", log_basedir, timestamp); if (len >= MAXPGPATH) pg_fatal("directory path for log files, %s/%s, is too long", - log_dir, timestamp); + logdir, timestamp); + + /* Create base directory (ignore if exists) */ + if (mkdir(log_basedir, S_IRWXU) < 0 && errno != EEXIST) + pg_fatal("could not create directory \"%s\": %m", log_basedir); - /* Create timestamp directory */ - make_dir(timestamp_dir); + /* Create BASE_DIR/$timestamp */ + if (mkdir(logdir, S_IRWXU) < 0) + pg_fatal("could not create directory \"%s\": %m", logdir); } /* @@ -1910,8 +1893,8 @@ 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, log_timestamp, SERVER_LOG_FILE_NAME); + if (opt->log_dir) + appendPQExpBuffer(pg_ctl_cmd, " -l %s/%s.log", logdir, SERVER_LOG_FILE_NAME); pg_createsub_log(PG_LOG_DEBUG, PG_LOG_PRIMARY, "pg_ctl command is: %s", pg_ctl_cmd->data); @@ -2668,18 +2651,6 @@ main(int argc, char **argv) } } - if (opt.log_dir != NULL) - { - char *internal_log_file; - - make_output_dirs(opt.log_dir); - internal_log_file = psprintf("%s/%s/%s.log", opt.log_dir, log_timestamp, - INTERNAL_LOG_FILE_NAME); - - if ((internal_log_file_fp = logfile_open(internal_log_file, "a")) == NULL) - pg_fatal("could not open log file \"%s\": %m", internal_log_file); - } - /* Validate that --all is not used with incompatible options */ if (opt.all_dbs) { @@ -2757,6 +2728,18 @@ 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); + + if ((internal_log_file_fp = logfile_open(internal_log_file, "a")) == NULL) + pg_fatal("could not open log file \"%s\": %m", internal_log_file); + } + if (dry_run) pg_createsub_log(PG_LOG_INFO, PG_LOG_PRIMARY, "Executing in dry-run mode.\n" -- 2.47.3