Attached three patches refactor the syslogger handling of file based
destinations a bit ... and then a lot.

First patch adds a new constant to represent both file based destinations.
This should make it easier to ensure additional destinations are handled in
"For all file destinations..." situations (e.g. when we add the jsonlog
destination).

Second patch refactors the file descriptor serialization and re-opening in
EXEC_BACKEND forking. Previously the code was duplicated for both stderr
and csvlog. Again, this should simplify adding new destinations as they'd
just invoke the same helper. There's an existing comment about not handling
failed opens in syslogger_parseArgs(...) and this patch doesn't fix that,
but it does provide a single location to do so in the future.

The third patch adds a new internal (to syslogger.c) structure,
FileLogDestination, for file based log destinations and modifies the
existing handling for syslogFile and csvlogFile to defer to a bunch of
helper functions. It also renames "syslogFile" to "stderr_file_info".
Working through this patch, it was initially confusing that the stderr log
file was named "syslogFile", the C file is named syslogger.c, and there's
an entirely separate syslog (the message logging API) destination. I think
this clears that up a bit.

The patches pass check-world on Linux. I haven't tested it on Windows but
it does pass check-world with EXEC_BACKEND defined to try out the forking
code paths. Definitely needs some review to ensure it's functionally
correct for the different log files.

I haven't tried splitting the csvlog code out or adding the new jsonlog
atop this yet. There's enough changes here that I'd like to get this
settled first.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
From bd5a4ff0435c721de3e7eb9b9207d9e96d79baf4 Mon Sep 17 00:00:00 2001
From: Sehrope Sarkuni <sehr...@jackdb.com>
Date: Thu, 16 Sep 2021 14:43:31 -0400
Subject: [PATCH 1/3] Add constant for list of log destinations that use files

---
 src/backend/postmaster/syslogger.c | 5 ++---
 src/include/utils/elog.h           | 3 +++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index bca3883572..bc546af7ff 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -420,7 +420,7 @@ SysLoggerMain(int argc, char *argv[])
 			 * was sent by pg_rotate_logfile() or "pg_ctl logrotate".
 			 */
 			if (!time_based_rotation && size_rotation_for == 0)
-				size_rotation_for = LOG_DESTINATION_STDERR | LOG_DESTINATION_CSVLOG;
+				size_rotation_for = LOG_DESTINATIONS_WITH_FILES;
 			logfile_rotate(time_based_rotation, size_rotation_for);
 		}
 
@@ -1465,8 +1465,7 @@ update_metainfo_datafile(void)
 	FILE	   *fh;
 	mode_t		oumask;
 
-	if (!(Log_destination & LOG_DESTINATION_STDERR) &&
-		!(Log_destination & LOG_DESTINATION_CSVLOG))
+	if (!(Log_destination & LOG_DESTINATIONS_WITH_FILES))
 	{
 		if (unlink(LOG_METAINFO_DATAFILE) < 0 && errno != ENOENT)
 			ereport(LOG,
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index f53607e12e..bea8b93da6 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -437,6 +437,9 @@ extern bool syslog_split_messages;
 #define LOG_DESTINATION_EVENTLOG 4
 #define LOG_DESTINATION_CSVLOG	 8
 
+/* Log destinations with file handles */
+#define LOG_DESTINATIONS_WITH_FILES (LOG_DESTINATION_CSVLOG | LOG_DESTINATION_STDERR)
+
 /* Other exported functions */
 extern void DebugFileOpen(void);
 extern char *unpack_sql_state(int sql_state);
-- 
2.17.1

From 653ea6865ce81de89bec4fc41f3c0be2933e6b2b Mon Sep 17 00:00:00 2001
From: Sehrope Sarkuni <sehr...@jackdb.com>
Date: Thu, 16 Sep 2021 15:57:00 -0400
Subject: [PATCH 2/3] Split out syslogger EXEC_BACKEND fd serialization and
 opening into helper functions

---
 src/backend/postmaster/syslogger.c | 109 ++++++++++++-----------------
 1 file changed, 44 insertions(+), 65 deletions(-)

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index bc546af7ff..4f0477794e 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -730,9 +730,23 @@ SysLogger_Start(void)
 	return 0;
 }
 
-
 #ifdef EXEC_BACKEND
 
+static long syslogger_get_fileno(FILE *file)
+{
+#ifndef WIN32
+	if (file != NULL)
+		return (long) fileno(file);
+	else
+		return -1;
+#else							/* WIN32 */
+	if (file != NULL)
+		return (long) _get_osfhandle(_fileno(file));
+	else
+		return 0;
+#endif							/* WIN32 */
+}
+
 /*
  * syslogger_forkexec() -
  *
@@ -751,34 +765,9 @@ syslogger_forkexec(void)
 	av[ac++] = NULL;			/* filled in by postmaster_forkexec */
 
 	/* static variables (those not passed by write_backend_variables) */
-#ifndef WIN32
-	if (syslogFile != NULL)
-		snprintf(filenobuf, sizeof(filenobuf), "%d",
-				 fileno(syslogFile));
-	else
-		strcpy(filenobuf, "-1");
-#else							/* WIN32 */
-	if (syslogFile != NULL)
-		snprintf(filenobuf, sizeof(filenobuf), "%ld",
-				 (long) _get_osfhandle(_fileno(syslogFile)));
-	else
-		strcpy(filenobuf, "0");
-#endif							/* WIN32 */
+	snprintf(filenobuf, sizeof(filenobuf), "%ld", syslogger_get_fileno(syslogFile));
 	av[ac++] = filenobuf;
-
-#ifndef WIN32
-	if (csvlogFile != NULL)
-		snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%d",
-				 fileno(csvlogFile));
-	else
-		strcpy(csvfilenobuf, "-1");
-#else							/* WIN32 */
-	if (csvlogFile != NULL)
-		snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld",
-				 (long) _get_osfhandle(_fileno(csvlogFile)));
-	else
-		strcpy(csvfilenobuf, "0");
-#endif							/* WIN32 */
+	snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld", syslogger_get_fileno(csvlogFile));
 	av[ac++] = csvfilenobuf;
 
 	av[ac] = NULL;
@@ -787,6 +776,31 @@ syslogger_forkexec(void)
 	return postmaster_forkexec(ac, av);
 }
 
+static FILE* syslogger_fdopen(int fd)
+{
+	FILE *file = NULL;
+
+#ifndef WIN32
+	if (fd != -1)
+	{
+		file = fdopen(fd, "a");
+		setvbuf(file, NULL, PG_IOLBF, 0);
+	}
+#else							/* WIN32 */
+	if (fd != 0)
+	{
+		fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
+		if (fd > 0)
+		{
+			file = fdopen(fd, "a");
+			setvbuf(file, NULL, PG_IOLBF, 0);
+		}
+	}
+#endif							/* WIN32 */
+
+	return file;
+}
+
 /*
  * syslogger_parseArgs() -
  *
@@ -795,8 +809,6 @@ syslogger_forkexec(void)
 static void
 syslogger_parseArgs(int argc, char *argv[])
 {
-	int			fd;
-
 	Assert(argc == 5);
 	argv += 3;
 
@@ -807,41 +819,8 @@ syslogger_parseArgs(int argc, char *argv[])
 	 * fails there's not a lot we can do to report the problem anyway.  As
 	 * coded, we'll just crash on a null pointer dereference after failure...
 	 */
-#ifndef WIN32
-	fd = atoi(*argv++);
-	if (fd != -1)
-	{
-		syslogFile = fdopen(fd, "a");
-		setvbuf(syslogFile, NULL, PG_IOLBF, 0);
-	}
-	fd = atoi(*argv++);
-	if (fd != -1)
-	{
-		csvlogFile = fdopen(fd, "a");
-		setvbuf(csvlogFile, NULL, PG_IOLBF, 0);
-	}
-#else							/* WIN32 */
-	fd = atoi(*argv++);
-	if (fd != 0)
-	{
-		fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
-		if (fd > 0)
-		{
-			syslogFile = fdopen(fd, "a");
-			setvbuf(syslogFile, NULL, PG_IOLBF, 0);
-		}
-	}
-	fd = atoi(*argv++);
-	if (fd != 0)
-	{
-		fd = _open_osfhandle(fd, _O_APPEND | _O_TEXT);
-		if (fd > 0)
-		{
-			csvlogFile = fdopen(fd, "a");
-			setvbuf(csvlogFile, NULL, PG_IOLBF, 0);
-		}
-	}
-#endif							/* WIN32 */
+	syslogFile = syslogger_fdopen(atoi(*argv++));
+	csvlogFile = syslogger_fdopen(atoi(*argv++));
 }
 #endif							/* EXEC_BACKEND */
 
-- 
2.17.1

From 99eb9e6b69a75fec3356696d10910c4cf89dae57 Mon Sep 17 00:00:00 2001
From: Sehrope Sarkuni <sehr...@jackdb.com>
Date: Thu, 16 Sep 2021 16:14:04 -0400
Subject: [PATCH 3/3] Refactor syslogger to consolidate common tasks for file
 based destinations

---
 src/backend/postmaster/syslogger.c | 415 ++++++++++++++---------------
 1 file changed, 199 insertions(+), 216 deletions(-)

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index 4f0477794e..0f5d3b7862 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -84,11 +84,28 @@ extern bool redirection_done;
 static pg_time_t next_rotation_time;
 static bool pipe_eof_seen = false;
 static bool rotation_disabled = false;
-static FILE *syslogFile = NULL;
-static FILE *csvlogFile = NULL;
 NON_EXEC_STATIC pg_time_t first_syslogger_file_time = 0;
-static char *last_file_name = NULL;
-static char *last_csv_file_name = NULL;
+
+typedef struct
+{
+	FILE *file;
+	char *last_file_name;
+	const int dest;
+	const char* name;
+	const char* suffix;
+} FileLogDestination;
+
+static FileLogDestination stderr_file_info = {NULL, NULL, LOG_DESTINATION_STDERR, "stderr", NULL};
+static FileLogDestination csvlog_file_info = {NULL, NULL, LOG_DESTINATION_CSVLOG, "csvlog", ".csv"};
+
+static inline void file_log_dest_initial_open(FileLogDestination *info);
+static inline void file_log_dest_initial_set_last_file_name(FileLogDestination *info);
+static inline void file_log_dest_check_rotate_for_open(FileLogDestination *info);
+static inline void file_log_dest_check_rotate_for_size(FileLogDestination *info, int *p_size_rotation_for);
+static inline void file_log_dest_close(FileLogDestination *info);
+static inline bool file_log_dest_should_rotate_for_size(FileLogDestination *info);
+static inline bool file_log_dest_rotate(FileLogDestination *info, pg_time_t fntime, bool time_based_rotation, int size_rotation_for);
+static bool inline file_log_dest_write_metadata(FileLogDestination *info, FILE *metadata_file);
 
 /*
  * Buffers for saving partial messages from different backends.
@@ -272,9 +289,8 @@ SysLoggerMain(int argc, char *argv[])
 	 * time because passing down just the pg_time_t is a lot cheaper than
 	 * passing a whole file path in the EXEC_BACKEND case.
 	 */
-	last_file_name = logfile_getname(first_syslogger_file_time, NULL);
-	if (csvlogFile != NULL)
-		last_csv_file_name = logfile_getname(first_syslogger_file_time, ".csv");
+	file_log_dest_initial_set_last_file_name(&stderr_file_info);
+	file_log_dest_initial_set_last_file_name(&csvlog_file_info);
 
 	/* remember active logfile parameters */
 	currentLogDir = pstrdup(Log_directory);
@@ -353,13 +369,7 @@ SysLoggerMain(int argc, char *argv[])
 				rotation_requested = true;
 			}
 
-			/*
-			 * Force a rotation if CSVLOG output was just turned on or off and
-			 * we need to open or close csvlogFile accordingly.
-			 */
-			if (((Log_destination & LOG_DESTINATION_CSVLOG) != 0) !=
-				(csvlogFile != NULL))
-				rotation_requested = true;
+			file_log_dest_check_rotate_for_open(&csvlog_file_info);
 
 			/*
 			 * If rotation time parameter changed, reset next rotation time,
@@ -399,18 +409,8 @@ SysLoggerMain(int argc, char *argv[])
 
 		if (!rotation_requested && Log_RotationSize > 0 && !rotation_disabled)
 		{
-			/* Do a rotation if file is too big */
-			if (ftell(syslogFile) >= Log_RotationSize * 1024L)
-			{
-				rotation_requested = true;
-				size_rotation_for |= LOG_DESTINATION_STDERR;
-			}
-			if (csvlogFile != NULL &&
-				ftell(csvlogFile) >= Log_RotationSize * 1024L)
-			{
-				rotation_requested = true;
-				size_rotation_for |= LOG_DESTINATION_CSVLOG;
-			}
+			file_log_dest_check_rotate_for_size(&stderr_file_info, &size_rotation_for);
+			file_log_dest_check_rotate_for_size(&csvlog_file_info, &size_rotation_for);
 		}
 
 		if (rotation_requested)
@@ -541,7 +541,6 @@ int
 SysLogger_Start(void)
 {
 	pid_t		sysloggerPid;
-	char	   *filename;
 
 	if (!Logging_collector)
 		return 0;
@@ -606,25 +605,10 @@ SysLogger_Start(void)
 	 */
 	first_syslogger_file_time = time(NULL);
 
-	filename = logfile_getname(first_syslogger_file_time, NULL);
-
-	syslogFile = logfile_open(filename, "a", false);
-
-	pfree(filename);
-
-	/*
-	 * Likewise for the initial CSV log file, if that's enabled.  (Note that
-	 * we open syslogFile even when only CSV output is nominally enabled,
-	 * since some code paths will write to syslogFile anyway.)
-	 */
-	if (Log_destination & LOG_DESTINATION_CSVLOG)
-	{
-		filename = logfile_getname(first_syslogger_file_time, ".csv");
-
-		csvlogFile = logfile_open(filename, "a", false);
-
-		pfree(filename);
-	}
+	/* stderr file will always be opened */
+	file_log_dest_initial_open(&stderr_file_info);
+	/* other files are opened if destination is enabled */
+	file_log_dest_initial_open(&csvlog_file_info);
 
 #ifdef EXEC_BACKEND
 	switch ((sysloggerPid = syslogger_forkexec()))
@@ -716,13 +700,8 @@ SysLogger_Start(void)
 			}
 
 			/* postmaster will never write the file(s); close 'em */
-			fclose(syslogFile);
-			syslogFile = NULL;
-			if (csvlogFile != NULL)
-			{
-				fclose(csvlogFile);
-				csvlogFile = NULL;
-			}
+			file_log_dest_close(&stderr_file_info);
+			file_log_dest_close(&csvlog_file_info);
 			return (int) sysloggerPid;
 	}
 
@@ -765,9 +744,9 @@ syslogger_forkexec(void)
 	av[ac++] = NULL;			/* filled in by postmaster_forkexec */
 
 	/* static variables (those not passed by write_backend_variables) */
-	snprintf(filenobuf, sizeof(filenobuf), "%ld", syslogger_get_fileno(syslogFile));
+	snprintf(filenobuf, sizeof(filenobuf), "%ld", syslogger_get_fileno(stderr_file_info->file));
 	av[ac++] = filenobuf;
-	snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld", syslogger_get_fileno(csvlogFile));
+	snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld", syslogger_get_fileno(csvlog_file_info->file));
 	av[ac++] = csvfilenobuf;
 
 	av[ac] = NULL;
@@ -819,8 +798,8 @@ syslogger_parseArgs(int argc, char *argv[])
 	 * fails there's not a lot we can do to report the problem anyway.  As
 	 * coded, we'll just crash on a null pointer dereference after failure...
 	 */
-	syslogFile = syslogger_fdopen(atoi(*argv++));
-	csvlogFile = syslogger_fdopen(atoi(*argv++));
+	stderr_file_info->file = syslogger_fdopen(atoi(*argv++));
+	csvlog_file_info->file = syslogger_fdopen(atoi(*argv++));
 }
 #endif							/* EXEC_BACKEND */
 
@@ -1050,6 +1029,26 @@ flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer)
 	*bytes_in_logbuffer = 0;
 }
 
+static inline FILE* get_syslogger_file(int dest)
+{
+	/*
+	 * If we're told to write to a logfile, but it's not open, dump the data
+	 * to stderr (which is always open) instead.  This can happen if the
+	 * output is enabled after postmaster start and we've been unable to open
+	 * outputs's file.  There are also race conditions during a parameter change
+	 * whereby backends might send us the output before we open the log file or
+	 * after we close it.  Writing formatted output to the regular log
+	 * file isn't great, but it beats dropping log output on the floor.
+	 *
+	 * Think not to improve this by trying to open a logfile on-the-fly.  Any
+	 * failure in that would lead to recursion.
+	 */
+	if (dest == LOG_DESTINATION_CSVLOG && csvlog_file_info.file != NULL) {
+		return csvlog_file_info.file;
+	}
+	return stderr_file_info.file;
+}
+
 
 /* --------------------------------
  *		logfile routines
@@ -1067,24 +1066,8 @@ void
 write_syslogger_file(const char *buffer, int count, int destination)
 {
 	int			rc;
-	FILE	   *logfile;
 
-	/*
-	 * If we're told to write to csvlogFile, but it's not open, dump the data
-	 * to syslogFile (which is always open) instead.  This can happen if CSV
-	 * output is enabled after postmaster start and we've been unable to open
-	 * csvlogFile.  There are also race conditions during a parameter change
-	 * whereby backends might send us CSV output before we open csvlogFile or
-	 * after we close it.  Writing CSV-formatted output to the regular log
-	 * file isn't great, but it beats dropping log output on the floor.
-	 *
-	 * Think not to improve this by trying to open csvlogFile on-the-fly.  Any
-	 * failure in that would lead to recursion.
-	 */
-	logfile = (destination == LOG_DESTINATION_CSVLOG &&
-			   csvlogFile != NULL) ? csvlogFile : syslogFile;
-
-	rc = fwrite(buffer, 1, count, logfile);
+	rc = fwrite(buffer, 1, count, get_syslogger_file(destination));
 
 	/*
 	 * Try to report any failure.  We mustn't use ereport because it would
@@ -1152,8 +1135,8 @@ pipeThread(void *arg)
 		 */
 		if (Log_RotationSize > 0)
 		{
-			if (ftell(syslogFile) >= Log_RotationSize * 1024L ||
-				(csvlogFile != NULL && ftell(csvlogFile) >= Log_RotationSize * 1024L))
+			if (file_log_dest_should_rotate_for_size(&stderr_file_info) ||
+				file_log_dest_should_rotate_for_size(&csvlog_file_info))
 				SetLatch(MyLatch);
 		}
 		LeaveCriticalSection(&sysloggerSection);
@@ -1224,10 +1207,7 @@ logfile_open(const char *filename, const char *mode, bool allow_errors)
 static void
 logfile_rotate(bool time_based_rotation, int size_rotation_for)
 {
-	char	   *filename;
-	char	   *csvfilename = NULL;
 	pg_time_t	fntime;
-	FILE	   *fh;
 
 	rotation_requested = false;
 
@@ -1240,124 +1220,15 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		fntime = next_rotation_time;
 	else
 		fntime = time(NULL);
-	filename = logfile_getname(fntime, NULL);
-	if (Log_destination & LOG_DESTINATION_CSVLOG)
-		csvfilename = logfile_getname(fntime, ".csv");
-
-	/*
-	 * Decide whether to overwrite or append.  We can overwrite if (a)
-	 * Log_truncate_on_rotation is set, (b) the rotation was triggered by
-	 * elapsed time and not something else, and (c) the computed file name is
-	 * different from what we were previously logging into.
-	 *
-	 * Note: last_file_name should never be NULL here, but if it is, append.
-	 */
-	if (time_based_rotation || (size_rotation_for & LOG_DESTINATION_STDERR))
-	{
-		if (Log_truncate_on_rotation && time_based_rotation &&
-			last_file_name != NULL &&
-			strcmp(filename, last_file_name) != 0)
-			fh = logfile_open(filename, "w", true);
-		else
-			fh = logfile_open(filename, "a", true);
-
-		if (!fh)
-		{
-			/*
-			 * ENFILE/EMFILE are not too surprising on a busy system; just
-			 * keep using the old file till we manage to get a new one.
-			 * Otherwise, assume something's wrong with Log_directory and stop
-			 * trying to create files.
-			 */
-			if (errno != ENFILE && errno != EMFILE)
-			{
-				ereport(LOG,
-						(errmsg("disabling automatic rotation (use SIGHUP to re-enable)")));
-				rotation_disabled = true;
-			}
-
-			if (filename)
-				pfree(filename);
-			if (csvfilename)
-				pfree(csvfilename);
-			return;
-		}
-
-		fclose(syslogFile);
-		syslogFile = fh;
-
-		/* instead of pfree'ing filename, remember it for next time */
-		if (last_file_name != NULL)
-			pfree(last_file_name);
-		last_file_name = filename;
-		filename = NULL;
-	}
-
-	/*
-	 * Same as above, but for csv file.  Note that if LOG_DESTINATION_CSVLOG
-	 * was just turned on, we might have to open csvlogFile here though it was
-	 * not open before.  In such a case we'll append not overwrite (since
-	 * last_csv_file_name will be NULL); that is consistent with the normal
-	 * rules since it's not a time-based rotation.
-	 */
-	if ((Log_destination & LOG_DESTINATION_CSVLOG) &&
-		(csvlogFile == NULL ||
-		 time_based_rotation || (size_rotation_for & LOG_DESTINATION_CSVLOG)))
-	{
-		if (Log_truncate_on_rotation && time_based_rotation &&
-			last_csv_file_name != NULL &&
-			strcmp(csvfilename, last_csv_file_name) != 0)
-			fh = logfile_open(csvfilename, "w", true);
-		else
-			fh = logfile_open(csvfilename, "a", true);
-
-		if (!fh)
-		{
-			/*
-			 * ENFILE/EMFILE are not too surprising on a busy system; just
-			 * keep using the old file till we manage to get a new one.
-			 * Otherwise, assume something's wrong with Log_directory and stop
-			 * trying to create files.
-			 */
-			if (errno != ENFILE && errno != EMFILE)
-			{
-				ereport(LOG,
-						(errmsg("disabling automatic rotation (use SIGHUP to re-enable)")));
-				rotation_disabled = true;
-			}
-
-			if (filename)
-				pfree(filename);
-			if (csvfilename)
-				pfree(csvfilename);
-			return;
-		}
-
-		if (csvlogFile != NULL)
-			fclose(csvlogFile);
-		csvlogFile = fh;
 
-		/* instead of pfree'ing filename, remember it for next time */
-		if (last_csv_file_name != NULL)
-			pfree(last_csv_file_name);
-		last_csv_file_name = csvfilename;
-		csvfilename = NULL;
-	}
-	else if (!(Log_destination & LOG_DESTINATION_CSVLOG) &&
-			 csvlogFile != NULL)
-	{
-		/* CSVLOG was just turned off, so close the old file */
-		fclose(csvlogFile);
-		csvlogFile = NULL;
-		if (last_csv_file_name != NULL)
-			pfree(last_csv_file_name);
-		last_csv_file_name = NULL;
+	if (!file_log_dest_rotate(&stderr_file_info, fntime, time_based_rotation, size_rotation_for) ||
+		!file_log_dest_rotate(&csvlog_file_info, fntime, time_based_rotation, size_rotation_for)) {
+		/* We failed to open a new log file so try again later */
+		return;
 	}
 
-	if (filename)
-		pfree(filename);
-	if (csvfilename)
-		pfree(csvfilename);
+	if (!(Log_destination & LOG_DESTINATION_CSVLOG))
+		file_log_dest_close(&csvlog_file_info);
 
 	update_metainfo_datafile();
 
@@ -1477,31 +1348,13 @@ update_metainfo_datafile(void)
 		return;
 	}
 
-	if (last_file_name && (Log_destination & LOG_DESTINATION_STDERR))
+	if (!file_log_dest_write_metadata(&stderr_file_info, fh) ||
+		!file_log_dest_write_metadata(&csvlog_file_info, fh))
 	{
-		if (fprintf(fh, "stderr %s\n", last_file_name) < 0)
-		{
-			ereport(LOG,
-					(errcode_for_file_access(),
-					 errmsg("could not write file \"%s\": %m",
-							LOG_METAINFO_DATAFILE_TMP)));
-			fclose(fh);
-			return;
-		}
+		fclose(fh);
+		return;
 	}
 
-	if (last_csv_file_name && (Log_destination & LOG_DESTINATION_CSVLOG))
-	{
-		if (fprintf(fh, "csvlog %s\n", last_csv_file_name) < 0)
-		{
-			ereport(LOG,
-					(errcode_for_file_access(),
-					 errmsg("could not write file \"%s\": %m",
-							LOG_METAINFO_DATAFILE_TMP)));
-			fclose(fh);
-			return;
-		}
-	}
 	fclose(fh);
 
 	if (rename(LOG_METAINFO_DATAFILE_TMP, LOG_METAINFO_DATAFILE) != 0)
@@ -1551,3 +1404,133 @@ sigUsr1Handler(SIGNAL_ARGS)
 
 	errno = save_errno;
 }
+
+static inline void file_log_dest_initial_open(FileLogDestination *info)
+{
+	/* We always open stderr. Otherwise only open if enabled. */
+	if (info->dest == LOG_DESTINATION_STDERR || (Log_destination & info->dest))
+	{
+		char *filename = logfile_getname(first_syslogger_file_time, info->suffix);
+		info->file = logfile_open(filename, "a", false);
+		pfree(filename);
+	}
+}
+
+static inline void file_log_dest_initial_set_last_file_name(FileLogDestination *info)
+{
+	if (info->file != NULL)
+		info->last_file_name = logfile_getname(first_syslogger_file_time, info->suffix);
+}
+
+static inline void file_log_dest_check_rotate_for_open(FileLogDestination *info)
+{
+	/*
+	* Force a rotation if a destination was just turned on or off and
+	* we need to open or close its file accordingly.
+	*/
+	if (((Log_destination & info->dest) != 0) != (info->file != NULL))
+		rotation_requested = true;
+}
+
+static inline void file_log_dest_check_rotate_for_size(FileLogDestination *info, int *p_size_rotation_for)
+{
+	if (info->file == NULL) {
+		return;
+	}
+	/* Do a rotation if file is too big */
+	if (ftell(info->file) >= Log_RotationSize * 1024L)
+	{
+		rotation_requested = true;
+		*p_size_rotation_for |= info->dest;
+	}
+}
+
+static inline void file_log_dest_close(FileLogDestination *info)
+{
+	if (info->file != NULL)
+	{
+		fclose(info->file);
+		info->file = NULL;
+	}
+	if (info->last_file_name != NULL)
+	{
+		pfree(info->last_file_name);
+		info->last_file_name = NULL;
+	}
+}
+
+static inline bool file_log_dest_should_rotate_for_size(FileLogDestination *info)
+{
+	if (info->file == NULL)
+		return false;
+
+	return ftell(info->file) >= Log_RotationSize * 1024L;
+}
+
+static inline bool file_log_dest_rotate(FileLogDestination *info, pg_time_t fntime, bool time_based_rotation, int size_rotation_for)
+{
+	/*
+	 * Decide whether to overwrite or append.  We can overwrite if (a)
+	 * Log_truncate_on_rotation is set, (b) the rotation was triggered by
+	 * elapsed time and not something else, and (c) the computed file name is
+	 * different from what we were previously logging into.
+	 *
+	 * Note: last_file_name should never be NULL here, but if it is, append.
+	 */
+	if (time_based_rotation || (size_rotation_for & info->dest))
+	{
+		FILE *file = NULL;
+		char *filename = logfile_getname(fntime, info->suffix);
+
+		if (Log_truncate_on_rotation
+			&& time_based_rotation
+			&& info->last_file_name != NULL
+			&& strcmp(filename, info->last_file_name) != 0)
+			file = logfile_open(filename, "w", true);
+		else
+			file = logfile_open(filename, "a", true);
+
+		if (file == NULL)
+		{
+			/*
+			 * ENFILE/EMFILE are not too surprising on a busy system; just
+			 * keep using the old file till we manage to get a new one.
+			 * Otherwise, assume something's wrong with Log_directory and stop
+			 * trying to create files.
+			 */
+			if (errno != ENFILE && errno != EMFILE)
+			{
+				ereport(LOG,
+						(errmsg("disabling automatic rotation (use SIGHUP to re-enable)")));
+				rotation_disabled = true;
+			}
+
+			if (filename)
+				pfree(filename);
+			return false;
+		}
+
+		file_log_dest_close(info);
+		info->file = file;
+		/* instead of pfree'ing filename, remember it for next time */
+		info->last_file_name = filename;
+	}
+
+	return true;
+}
+
+static bool inline file_log_dest_write_metadata(FileLogDestination *info, FILE *metadata_file)
+{
+	if (info->last_file_name != NULL && (Log_destination & info->dest))
+	{
+		if (fprintf(metadata_file, "%s %s\n", info->name, info->last_file_name) < 0)
+		{
+			ereport(LOG,
+					(errcode_for_file_access(),
+					 errmsg("could not write file \"%s\": %m",
+							LOG_METAINFO_DATAFILE_TMP)));
+			return false;
+		}
+	}
+	return true;
+}
-- 
2.17.1

Reply via email to