From 0da9a9de5ab07e90e59ce496a39bf1e48e354170 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
Date: Wed, 11 Mar 2020 02:58:08 +0000
Subject: [PATCH v2 07/12] Add SysLoggerType to subprocess struct

This commit introduces two new fields to the subprocess struct:

1) keep_postmaster_memcontext allows subprocesses to specify whether or
not the PostmasterMemoryContext should be kept around after a fork.

2) postmaster_main allows postmaster to gain control to do some
necessary processing after a successful fork.
---
 src/backend/postmaster/postmaster.c |  39 +++--
 src/backend/postmaster/subprocess.c |  56 +++++--
 src/backend/postmaster/syslogger.c  | 227 +++++++++++-----------------
 src/include/postmaster/subprocess.h |  18 ++-
 src/include/postmaster/syslogger.h  |   5 +-
 5 files changed, 177 insertions(+), 168 deletions(-)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 88d010ceb1..f724727b1e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1083,7 +1083,7 @@ PostmasterMain(int argc, char *argv[])
 	/*
 	 * If enabled, start up syslogger collection subprocess
 	 */
-	SysLoggerPID = SysLogger_Start();
+	SysLoggerPID = StartSubprocess(SysLoggerType);
 
 	/*
 	 * Reset whereToSendOutput from DestDebug (its starting state) to
@@ -1739,7 +1739,7 @@ ServerLoop(void)
 
 		/* If we have lost the log collector, try to start a new one */
 		if (SysLoggerPID == 0 && Logging_collector)
-			SysLoggerPID = SysLogger_Start();
+			SysLoggerPID = StartSubprocess(SysLoggerType);
 
 		/*
 		 * If no background writer process is running, and we are not in a
@@ -3228,7 +3228,7 @@ reaper(SIGNAL_ARGS)
 		{
 			SysLoggerPID = 0;
 			/* for safety's sake, launch new logger *first* */
-			SysLoggerPID = SysLogger_Start();
+			SysLoggerPID = StartSubprocess(SysLoggerType);
 			if (!EXIT_STATUS_0(exitstatus))
 				LogChildExit(LOG, _("system logger process"),
 							 pid, exitstatus);
@@ -5063,12 +5063,6 @@ SubPostmasterMain(int argc, char *argv[])
 
 		StartBackgroundWorker();
 	}
-	if (strcmp(argv[1], "--forklog") == 0)
-	{
-		/* Do not want to attach to shared memory */
-
-		SysLoggerMain(argc, argv);	/* does not return */
-	}
 
 	if (MySubprocess->needs_aux_proc)
 	{
@@ -5446,6 +5440,15 @@ StartSubprocess(SubprocessType type)
 	snprintf(typebuf, sizeof(typebuf), "-x%d", type);
 	argv[argc++] = typebuf;
 
+	/* Prep subprocess for forking */
+	if (MySubprocess->fork_prep)
+	{
+		if (MySubprocess->fork_prep(argc, argv))
+		{
+			return 0;
+		}
+	}
+
 	argv[argc] = NULL;
 	Assert(argc < lengthof(argv));
 
@@ -5461,10 +5464,16 @@ StartSubprocess(SubprocessType type)
 		/* Close the postmaster's sockets */
 		ClosePostmasterPorts(false);
 
-		/* Release postmaster's working memory context */
-		MemoryContextSwitchTo(TopMemoryContext);
-		MemoryContextDelete(PostmasterContext);
-		PostmasterContext = NULL;
+		/*
+		 * Release postmaster's working memory context if the subprocess doesn't
+		 * need it
+		 */
+		if (!MySubprocess->keep_postmaster_memcontext)
+		{
+			MemoryContextSwitchTo(TopMemoryContext);
+			MemoryContextDelete(PostmasterContext);
+			PostmasterContext = NULL;
+		}
 
 		/*
 		 * Drop connection to postmaster's shared memory if the subprocess
@@ -5508,6 +5517,10 @@ StartSubprocess(SubprocessType type)
 		return 0;
 	}
 
+	/* Some subprocesses need to do more work in postmaster. */
+	if (MySubprocess->postmaster_main)
+		MySubprocess->postmaster_main(argc, argv);
+
 	/*
 	 * in parent, successful fork
 	 */
diff --git a/src/backend/postmaster/subprocess.c b/src/backend/postmaster/subprocess.c
index f9d71a627c..f9d07de4ae 100644
--- a/src/backend/postmaster/subprocess.c
+++ b/src/backend/postmaster/subprocess.c
@@ -19,6 +19,7 @@
 #include "postmaster/postmaster.h"
 #include "postmaster/startup.h"
 #include "postmaster/subprocess.h"
+#include "postmaster/syslogger.h"
 #include "postmaster/walwriter.h"
 #include "replication/walreceiver.h"
 
@@ -31,99 +32,132 @@ static PgSubprocess process_types[] = {
 		.desc = "checker",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = CheckerModeMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "boot",
 		.desc = "bootstrap",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = BootstrapModeMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "boot",
 		.desc = "startup",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = StartupProcessMain,
-		.fork_failure = StartupForkFailure
+		.fork_failure = StartupForkFailure,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "boot",
 		.desc = "background writer",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = BackgroundWriterMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "boot",
 		.desc = "checkpointer",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = CheckpointerMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "boot",
 		.desc = "wal writer",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = WalWriterMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "boot",
 		.desc = "wal receiver",
 		.needs_aux_proc = true,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = WalReceiverMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "avlauncher",
 		.desc = "autovacuum launcher",
 		.needs_aux_proc = false,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = AutoVacLauncherMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "avworker",
 		.desc = "autovacuum worker",
 		.needs_aux_proc = false,
 		.needs_shmem = true,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = NULL,
 		.entrypoint = AutoVacWorkerMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "col",
 		.desc = "statistics collector",
 		.needs_aux_proc = false,
 		.needs_shmem = false,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = PgstatCollectorPrep,
 		.entrypoint = PgstatCollectorMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
 	},
 	{
 		.name = "arch",
 		.desc = "archiver",
 		.needs_aux_proc = false,
 		.needs_shmem = false,
+		.keep_postmaster_memcontext = false,
 		.fork_prep = PgArchiverPrep,
 		.entrypoint = PgArchiverMain,
-		.fork_failure = NULL
+		.fork_failure = NULL,
+		.postmaster_main = NULL
+	},
+	{
+		.name = "log",
+		.desc = "syslogger",
+		.needs_aux_proc = false,
+		.needs_shmem = false,
+		.keep_postmaster_memcontext = true,
+		.fork_prep = SysLoggerPrep,
+		.entrypoint = SysLoggerMain,
+		.fork_failure = NULL,
+		.postmaster_main = SysLoggerPostmasterMain
 	}
 };
 
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index ffcb54968f..e56b0a77dd 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -38,8 +38,8 @@
 #include "nodes/pg_list.h"
 #include "pgstat.h"
 #include "pgtime.h"
-#include "postmaster/fork_process.h"
 #include "postmaster/postmaster.h"
+#include "postmaster/subprocess.h"
 #include "postmaster/syslogger.h"
 #include "storage/dsm.h"
 #include "storage/fd.h"
@@ -129,10 +129,8 @@ static volatile sig_atomic_t rotation_requested = false;
 
 /* Local subroutines */
 #ifdef EXEC_BACKEND
-static pid_t syslogger_forkexec(void);
 static void syslogger_parseArgs(int argc, char *argv[]);
 #endif
-NON_EXEC_STATIC void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn();
 static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
 static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
 static FILE *logfile_open(const char *filename, const char *mode,
@@ -153,7 +151,7 @@ static void update_metainfo_datafile(void);
  * Main entry point for syslogger process
  * argc/argv parameters are valid only in EXEC_BACKEND case.
  */
-NON_EXEC_STATIC void
+void
 SysLoggerMain(int argc, char *argv[])
 {
 #ifndef WIN32
@@ -537,13 +535,17 @@ SysLoggerMain(int argc, char *argv[])
  * Postmaster subroutine to start a syslogger subprocess.
  */
 int
-SysLogger_Start(void)
+SysLoggerPrep(int argc, char *argv[])
 {
-	pid_t		sysloggerPid;
 	char	   *filename;
+#ifdef EXEC_BACKEND
+	char		filenobuf[32];
+	char		csvfilenobuf[32];
+#endif
 
+	/* Tell postmaster to stop forking */
 	if (!Logging_collector)
-		return 0;
+		return -1;
 
 	/*
 	 * If first time through, create the pipe which will receive stderr
@@ -626,129 +628,6 @@ SysLogger_Start(void)
 	}
 
 #ifdef EXEC_BACKEND
-	switch ((sysloggerPid = syslogger_forkexec()))
-#else
-	switch ((sysloggerPid = fork_process()))
-#endif
-	{
-		case -1:
-			ereport(LOG,
-					(errmsg("could not fork system logger: %m")));
-			return 0;
-
-#ifndef EXEC_BACKEND
-		case 0:
-			/* in postmaster child ... */
-			InitPostmasterChild();
-
-			/* Close the postmaster's sockets */
-			ClosePostmasterPorts(true);
-
-			/* Drop our connection to postmaster's shared memory, as well */
-			dsm_detach_all();
-			PGSharedMemoryDetach();
-
-			/* do the work */
-			SysLoggerMain(0, NULL);
-			break;
-#endif
-
-		default:
-			/* success, in postmaster */
-
-			/* now we redirect stderr, if not done already */
-			if (!redirection_done)
-			{
-#ifdef WIN32
-				int			fd;
-#endif
-
-				/*
-				 * Leave a breadcrumb trail when redirecting, in case the user
-				 * forgets that redirection is active and looks only at the
-				 * original stderr target file.
-				 */
-				ereport(LOG,
-						(errmsg("redirecting log output to logging collector process"),
-						 errhint("Future log output will appear in directory \"%s\".",
-								 Log_directory)));
-
-#ifndef WIN32
-				fflush(stdout);
-				if (dup2(syslogPipe[1], fileno(stdout)) < 0)
-					ereport(FATAL,
-							(errcode_for_file_access(),
-							 errmsg("could not redirect stdout: %m")));
-				fflush(stderr);
-				if (dup2(syslogPipe[1], fileno(stderr)) < 0)
-					ereport(FATAL,
-							(errcode_for_file_access(),
-							 errmsg("could not redirect stderr: %m")));
-				/* Now we are done with the write end of the pipe. */
-				close(syslogPipe[1]);
-				syslogPipe[1] = -1;
-#else
-
-				/*
-				 * open the pipe in binary mode and make sure stderr is binary
-				 * after it's been dup'ed into, to avoid disturbing the pipe
-				 * chunking protocol.
-				 */
-				fflush(stderr);
-				fd = _open_osfhandle((intptr_t) syslogPipe[1],
-									 _O_APPEND | _O_BINARY);
-				if (dup2(fd, _fileno(stderr)) < 0)
-					ereport(FATAL,
-							(errcode_for_file_access(),
-							 errmsg("could not redirect stderr: %m")));
-				close(fd);
-				_setmode(_fileno(stderr), _O_BINARY);
-
-				/*
-				 * Now we are done with the write end of the pipe.
-				 * CloseHandle() must not be called because the preceding
-				 * close() closes the underlying handle.
-				 */
-				syslogPipe[1] = 0;
-#endif
-				redirection_done = true;
-			}
-
-			/* postmaster will never write the file(s); close 'em */
-			fclose(syslogFile);
-			syslogFile = NULL;
-			if (csvlogFile != NULL)
-			{
-				fclose(csvlogFile);
-				csvlogFile = NULL;
-			}
-			return (int) sysloggerPid;
-	}
-
-	/* we should never reach here */
-	return 0;
-}
-
-
-#ifdef EXEC_BACKEND
-
-/*
- * syslogger_forkexec() -
- *
- * Format up the arglist for, then fork and exec, a syslogger process
- */
-static pid_t
-syslogger_forkexec(void)
-{
-	char	   *av[10];
-	int			ac = 0;
-	char		filenobuf[32];
-	char		csvfilenobuf[32];
-
-	av[ac++] = "postgres";
-	av[ac++] = "--forklog";
-	av[ac++] = NULL;			/* filled in by postmaster_forkexec */
-
 	/* static variables (those not passed by write_backend_variables) */
 #ifndef WIN32
 	if (syslogFile != NULL)
@@ -763,7 +642,7 @@ syslogger_forkexec(void)
 	else
 		strcpy(filenobuf, "0");
 #endif							/* WIN32 */
-	av[ac++] = filenobuf;
+	argv[argc++] = filenobuf;
 
 #ifndef WIN32
 	if (csvlogFile != NULL)
@@ -778,14 +657,92 @@ syslogger_forkexec(void)
 	else
 		strcpy(csvfilenobuf, "0");
 #endif							/* WIN32 */
-	av[ac++] = csvfilenobuf;
+	argv[argc++] = csvfilenobuf;
 
-	av[ac] = NULL;
-	Assert(ac < lengthof(av));
+#endif							/* EXEC_BACKEND */
 
-	return postmaster_forkexec(ac, av);
+	return 0;
 }
 
+/*
+ * SysLoggerPostmasterMain
+ *
+ * Control function for subprocess parent (postmaster) after a successful fork.
+ */
+void
+SysLoggerPostmasterMain(int argc, char *argv[])
+{
+	/* success, in postmaster */
+
+	/* now we redirect stderr, if not done already */
+	if (!redirection_done)
+	{
+#ifdef WIN32
+		int			fd;
+#endif
+
+		/*
+		 * Leave a breadcrumb trail when redirecting, in case the user
+		 * forgets that redirection is active and looks only at the
+		 * original stderr target file.
+		 */
+		ereport(LOG,
+				(errmsg("redirecting log output to logging collector process"),
+				 errhint("Future log output will appear in directory \"%s\".",
+						 Log_directory)));
+
+#ifndef WIN32
+		fflush(stdout);
+		if (dup2(syslogPipe[1], fileno(stdout)) < 0)
+			ereport(FATAL,
+					(errcode_for_file_access(),
+					 errmsg("could not redirect stdout: %m")));
+		fflush(stderr);
+		if (dup2(syslogPipe[1], fileno(stderr)) < 0)
+			ereport(FATAL,
+					(errcode_for_file_access(),
+					 errmsg("could not redirect stderr: %m")));
+		/* Now we are done with the write end of the pipe. */
+		close(syslogPipe[1]);
+		syslogPipe[1] = -1;
+#else
+
+		/*
+		 * open the pipe in binary mode and make sure stderr is binary
+		 * after it's been dup'ed into, to avoid disturbing the pipe
+		 * chunking protocol.
+		 */
+		fflush(stderr);
+		fd = _open_osfhandle((intptr_t) syslogPipe[1],
+							 _O_APPEND | _O_BINARY);
+		if (dup2(fd, _fileno(stderr)) < 0)
+			ereport(FATAL,
+					(errcode_for_file_access(),
+					 errmsg("could not redirect stderr: %m")));
+		close(fd);
+		_setmode(_fileno(stderr), _O_BINARY);
+
+		/*
+		 * Now we are done with the write end of the pipe.
+		 * CloseHandle() must not be called because the preceding
+		 * close() closes the underlying handle.
+		 */
+		syslogPipe[1] = 0;
+#endif
+		redirection_done = true;
+	}
+
+	/* postmaster will never write the file(s); close 'em */
+	fclose(syslogFile);
+	syslogFile = NULL;
+	if (csvlogFile != NULL)
+	{
+		fclose(csvlogFile);
+		csvlogFile = NULL;
+	}
+}
+
+#ifdef EXEC_BACKEND
 /*
  * syslogger_parseArgs() -
  *
diff --git a/src/include/postmaster/subprocess.h b/src/include/postmaster/subprocess.h
index 3f00f904dc..d0d98a5d69 100644
--- a/src/include/postmaster/subprocess.h
+++ b/src/include/postmaster/subprocess.h
@@ -27,6 +27,7 @@ typedef enum
 	AutoVacuumWorkerType,
 	PgstatCollectorType,
 	PgArchiverType,
+	SysLoggerType,
 
 	NUMSUBPROCESSTYPES			/* Must be last! */
 } SubprocessType;
@@ -34,19 +35,22 @@ typedef enum
 typedef int		(*SubprocessPrep) (int argc, char *argv[]);
 typedef void	(*SubprocessEntryPoint) (int argc, char *argv[]);
 typedef bool	(*SubprocessForkFailure) (int fork_errno);
+typedef void	(*SubprocessPostmasterMain) (int argc, char *argv[]);
 
 /* Current subprocess initializer */
 extern void InitMySubprocess(SubprocessType type);
 
 typedef struct PgSubprocess
 {
-	const char			   *name;
-	const char			   *desc;
-	bool					needs_aux_proc;
-	bool					needs_shmem;
-	SubprocessPrep			fork_prep;
-	SubprocessEntryPoint	entrypoint;
-	SubprocessForkFailure	fork_failure;
+	const char				   *name;
+	const char				   *desc;
+	bool						needs_aux_proc;
+	bool						needs_shmem;
+	bool						keep_postmaster_memcontext;
+	SubprocessPrep				fork_prep;
+	SubprocessEntryPoint		entrypoint;
+	SubprocessForkFailure		fork_failure;
+	SubprocessPostmasterMain	postmaster_main;
 } PgSubprocess;
 
 extern SubprocessType				MySubprocessType;
diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h
index f611bd1411..a498c73121 100644
--- a/src/include/postmaster/syslogger.h
+++ b/src/include/postmaster/syslogger.h
@@ -81,9 +81,10 @@ extern int	SysLogger_Start(void);
 
 extern void write_syslogger_file(const char *buffer, int count, int dest);
 
-#ifdef EXEC_BACKEND
+/* Subprocess startup functions */
+extern int SysLoggerPrep(int argc, char *argv[]);
 extern void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn();
-#endif
+extern void SysLoggerPostmasterMain(int argc, char *argv[]);
 
 extern bool CheckLogrotateSignal(void);
 extern void RemoveLogrotateSignalFiles(void);
-- 
2.17.0

