v11-0003-Address-comments-from-Hayato-Kuroda.patch

From 1f72429ebf20958973edb84822c2fbbf603165f5 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <kuroda.hayato@fujitsu.com>
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