Subject: [PATCH v39 3/7] Make archiver process an auxiliary process

From: Kyotaro Horiguchi <[email-masked]>
Date: Fri, 13 Mar 2020 16:59:38 +0900

This is a preliminary patch for shared-memory based stats collector.

Archiver process must be a auxiliary process since it uses shared
memory after stats data was moved into shared-memory. Make the process
an auxiliary process in order to make it work.
---
 src/backend/access/transam/xlogarchive.c |   6 +-
 src/backend/bootstrap/bootstrap.c        |  22 ++--
 src/backend/postmaster/pgarch.c          | 130 +++--------------------
 src/backend/postmaster/postmaster.c      |  50 +++++----
 src/backend/storage/lmgr/proc.c          |   1 +
 src/include/access/xlog.h                |   3 +
 src/include/access/xlogarchive.h         |   1 +
 src/include/miscadmin.h                  |   2 +
 src/include/postmaster/pgarch.h          |   4 +-
 src/include/storage/pmsignal.h           |   1 -
 src/include/storage/proc.h               |   3 +
 11 files changed, 69 insertions(+), 154 deletions(-)

diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c
index cae93ab69d..6908bec2f9 100644
--- a/src/backend/access/transam/xlogarchive.c
+++ b/src/backend/access/transam/xlogarchive.c
@@ -29,7 +29,9 @@
 #include "storage/fd.h"
 #include "storage/ipc.h"
 #include "storage/lwlock.h"
+#include "storage/latch.h"
 #include "storage/pmsignal.h"
+#include "storage/proc.h"
 
 /*
  * Attempt to retrieve the specified file from off-line archival storage.
@@ -489,8 +491,8 @@ XLogArchiveNotify(const char *xlog)
 	}
 
 	/* Notify archiver that it's got something to do */
-	if (IsUnderPostmaster)
-		SendPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER);
+	if (IsUnderPostmaster && ProcGlobal->archiverLatch)
+		SetLatch(ProcGlobal->archiverLatch);
 }
 
 /*
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 76b2f5066f..81bfaea869 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -317,6 +317,9 @@ AuxiliaryProcessMain(int argc, char *argv[])
 		case StartupProcess:
 			MyBackendType = B_STARTUP;
 			break;
+		case ArchiverProcess:
+			MyBackendType = B_ARCHIVER;
+			break;
 		case BgWriterProcess:
 			MyBackendType = B_BG_WRITER;
 			break;
@@ -437,30 +440,29 @@ AuxiliaryProcessMain(int argc, char *argv[])
 			proc_exit(1);		/* should never return */
 
 		case StartupProcess:
-			/* don't set signals, startup process has its own agenda */
 			StartupProcessMain();
-			proc_exit(1);		/* should never return */
+			proc_exit(1);
+
+		case ArchiverProcess:
+			PgArchiverMain();
+			proc_exit(1);
 
 		case BgWriterProcess:
-			/* don't set signals, bgwriter has its own agenda */
 			BackgroundWriterMain();
-			proc_exit(1);		/* should never return */
+			proc_exit(1);
 
 		case CheckpointerProcess:
-			/* don't set signals, checkpointer has its own agenda */
 			CheckpointerMain();
-			proc_exit(1);		/* should never return */
+			proc_exit(1);
 
 		case WalWriterProcess:
-			/* don't set signals, walwriter has its own agenda */
 			InitXLOGAccess();
 			WalWriterMain();
-			proc_exit(1);		/* should never return */
+			proc_exit(1);
 
 		case WalReceiverProcess:
-			/* don't set signals, walreceiver has its own agenda */
 			WalReceiverMain();
-			proc_exit(1);		/* should never return */
+			proc_exit(1);
 
 		default:
 			elog(PANIC, "unrecognized process type: %d", (int) MyAuxProcType);
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index ed1b65358d..e3a520def9 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -48,6 +48,7 @@
 #include "storage/latch.h"
 #include "storage/pg_shmem.h"
 #include "storage/pmsignal.h"
+#include "storage/procsignal.h"
 #include "utils/guc.h"
 #include "utils/ps_status.h"
 
@@ -78,13 +79,11 @@
  * Local data
  * ----------
  */
-static time_t last_pgarch_start_time;
 static time_t last_sigterm_time = 0;
 
 /*
  * Flags set by interrupt handlers for later service in the main loop.
  */
-static volatile sig_atomic_t wakened = false;
 static volatile sig_atomic_t ready_to_stop = false;
 
 /* ----------
@@ -95,8 +94,6 @@ static volatile sig_atomic_t ready_to_stop = false;
 static pid_t pgarch_forkexec(void);
 #endif
 
-NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-static void pgarch_waken(SIGNAL_ARGS);
 static void pgarch_waken_stop(SIGNAL_ARGS);
 static void pgarch_MainLoop(void);
 static void pgarch_ArchiverCopyLoop(void);
@@ -110,75 +107,6 @@ static void pgarch_archiveDone(char *xlog);
  * ------------------------------------------------------------
  */
 
-/*
- * pgarch_start
- *
- *	Called from postmaster at startup or after an existing archiver
- *	died.  Attempt to fire up a fresh archiver process.
- *
- *	Returns PID of child process, or 0 if fail.
- *
- *	Note: if fail, we will be called again from the postmaster main loop.
- */
-int
-pgarch_start(void)
-{
-	time_t		curtime;
-	pid_t		pgArchPid;
-
-	/*
-	 * Do nothing if no archiver needed
-	 */
-	if (!XLogArchivingActive())
-		return 0;
-
-	/*
-	 * Do nothing if too soon since last archiver start.  This is a safety
-	 * valve to protect against continuous respawn attempts if the archiver is
-	 * dying immediately at launch. Note that since we will be re-called from
-	 * the postmaster main loop, we will get another chance later.
-	 */
-	curtime = time(NULL);
-	if ((unsigned int) (curtime - last_pgarch_start_time) <
-		(unsigned int) PGARCH_RESTART_INTERVAL)
-		return 0;
-	last_pgarch_start_time = curtime;
-
-#ifdef EXEC_BACKEND
-	switch ((pgArchPid = pgarch_forkexec()))
-#else
-	switch ((pgArchPid = fork_process()))
-#endif
-	{
-		case -1:
-			ereport(LOG,
-					(errmsg("could not fork archiver: %m")));
-			return 0;
-
-#ifndef EXEC_BACKEND
-		case 0:
-			/* in postmaster child ... */
-			InitPostmasterChild();
-
-			/* Close the postmaster's sockets */
-			ClosePostmasterPorts(false);
-
-			/* Drop our connection to postmaster's shared memory, as well */
-			dsm_detach_all();
-			PGSharedMemoryDetach();
-
-			PgArchiverMain(0, NULL);
-			break;
-#endif
-
-		default:
-			return (int) pgArchPid;
-	}
-
-	/* shouldn't get here */
-	return 0;
-}
-
 /* ------------------------------------------------------------
  * Local functions called by archiver follow
  * ------------------------------------------------------------
@@ -212,14 +140,9 @@ pgarch_forkexec(void)
 #endif							/* EXEC_BACKEND */

-/*
- * PgArchiverMain
- *
- *	The argc/argv parameters are valid only in EXEC_BACKEND case.  However,
- *	since we don't use 'em, it hardly matters...
- */
-NON_EXEC_STATIC void
-PgArchiverMain(int argc, char *argv[])
+/* Main entry point for archiver process */
+void
+PgArchiverMain(void)
 {
 	/*
 	 * Ignore all signals usually bound to some action in the postmaster,
@@ -231,33 +154,26 @@ PgArchiverMain(int argc, char *argv[])
 	/* SIGQUIT handler was already set up by InitPostmasterChild */
 	pqsignal(SIGALRM, SIG_IGN);
 	pqsignal(SIGPIPE, SIG_IGN);
-	pqsignal(SIGUSR1, pgarch_waken);
+	pqsignal(SIGUSR1, procsignal_sigusr1_handler);
 	pqsignal(SIGUSR2, pgarch_waken_stop);
+
 	/* Reset some signals that are accepted by postmaster but not here */
 	pqsignal(SIGCHLD, SIG_DFL);
+
+	/* Unblock signals (they were blocked when the postmaster forked us) */
 	PG_SETMASK(&UnBlockSig);
 
-	MyBackendType = B_ARCHIVER;
-	init_ps_display(NULL);
+	/*
+	 * Advertise our latch that backends can use to wake us up while we're
+	 * sleeping.
+	 */
+	ProcGlobal->archiverLatch = &MyProc->procLatch;
 
 	pgarch_MainLoop();
 
 	exit(0);
 }
 
-/* SIGUSR1 signal handler for archiver process */
-static void
-pgarch_waken(SIGNAL_ARGS)
-{
-	int			save_errno = errno;
-
-	/* set flag that there is work to be done */
-	wakened = true;
-	SetLatch(MyLatch);
-
-	errno = save_errno;
-}
-
 /* SIGUSR2 signal handler for archiver process */
 static void
 pgarch_waken_stop(SIGNAL_ARGS)
@@ -282,14 +198,6 @@ pgarch_MainLoop(void)
 	pg_time_t	last_copy_time = 0;
 	bool		time_to_stop;
 
-	/*
-	 * We run the copy loop immediately upon entry, in case there are
-	 * unarchived files left over from a previous database run (or maybe the
-	 * archiver died unexpectedly).  After that we wait for a signal or
-	 * timeout before doing more.
-	 */
-	wakened = true;
-
 	/*
 	 * There shouldn't be anything for the archiver to do except to wait for a
 	 * signal ... however, the archiver exists to protect our data, so she
@@ -328,12 +236,8 @@ pgarch_MainLoop(void)
 		}
 
 		/* Do what we're here for */
-		if (wakened || time_to_stop)
-		{
-			wakened = false;
-			pgarch_ArchiverCopyLoop();
-			last_copy_time = time(NULL);
-		}
+		pgarch_ArchiverCopyLoop();
+		last_copy_time = time(NULL);
 
 		/*
 		 * Sleep until a signal is received, or until a poll is forced by
@@ -354,13 +258,9 @@ pgarch_MainLoop(void)
 							   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
 							   timeout * 1000L,
 							   WAIT_EVENT_ARCHIVER_MAIN);
-				if (rc & WL_TIMEOUT)
-					wakened = true;
 				if (rc & WL_POSTMASTER_DEATH)
 					time_to_stop = true;
 			}
-			else
-				wakened = true;
 		}
 
 		/*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 959e3b8873..b811c961a6 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -555,6 +555,7 @@ static void ShmemBackendArrayRemove(Backend *bn);
 #endif							/* EXEC_BACKEND */
 
 #define StartupDataBase()		StartChildProcess(StartupProcess)
+#define StartArchiver()			StartChildProcess(ArchiverProcess)
 #define StartBackgroundWriter() StartChildProcess(BgWriterProcess)
 #define StartCheckpointer()		StartChildProcess(CheckpointerProcess)
 #define StartWalWriter()		StartChildProcess(WalWriterProcess)
@@ -1800,7 +1801,7 @@ ServerLoop(void)
 
 		/* If we have lost the archiver, try to start a new one. */
 		if (PgArchPID == 0 && PgArchStartupAllowed())
-			PgArchPID = pgarch_start();
+			PgArchPID = StartArchiver();
 
 		/* If we need to signal the autovacuum launcher, do so now */
 		if (avlauncher_needs_signal)
@@ -3054,7 +3055,7 @@ reaper(SIGNAL_ARGS)
 			if (!IsBinaryUpgrade && AutoVacuumingActive() && AutoVacPID == 0)
 				AutoVacPID = StartAutoVacLauncher();
 			if (PgArchStartupAllowed() && PgArchPID == 0)
-				PgArchPID = pgarch_start();
+				PgArchPID = StartArchiver();
 			if (PgStatPID == 0)
 				PgStatPID = pgstat_start();
 
@@ -3189,20 +3190,16 @@ reaper(SIGNAL_ARGS)
 		}
 
 		/*
-		 * Was it the archiver?  If so, just try to start a new one; no need
-		 * to force reset of the rest of the system.  (If fail, we'll try
-		 * again in future cycles of the main loop.).  Unless we were waiting
-		 * for it to shut down; don't restart it in that case, and
-		 * PostmasterStateMachine() will advance to the next shutdown step.
+		 * Was it the archiver?  Normal exit can be ignored; we'll start a new
+		 * one at the next iteration of the postmaster's main loop, if
+		 * necessary. Any other exit condition is treated as a crash.
 		 */
 		if (pid == PgArchPID)
 		{
 			PgArchPID = 0;
 			if (!EXIT_STATUS_0(exitstatus))
-				LogChildExit(LOG, _("archiver process"),
-							 pid, exitstatus);
-			if (PgArchStartupAllowed())
-				PgArchPID = pgarch_start();
+				HandleChildCrash(pid, exitstatus,
+								 _("archiver process"));
 			continue;
 		}
 
@@ -3450,7 +3447,7 @@ CleanupBackend(int pid,
 
 /*
  * HandleChildCrash -- cleanup after failed backend, bgwriter, checkpointer,
- * walwriter, autovacuum, or background worker.
+ * walwriter, autovacuum, archiver or background worker.
  *
  * The objectives here are to clean up our local state about the child
  * process, and to signal all other remaining children to quickdie.
@@ -3655,6 +3652,18 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
 		signal_child(AutoVacPID, (SendStop ? SIGSTOP : SIGQUIT));
 	}
 
+	/* Take care of the archiver too */
+	if (pid == PgArchPID)
+		PgArchPID = 0;
+	else if (PgArchPID != 0 && take_action)
+	{
+		ereport(DEBUG2,
+				(errmsg_internal("sending %s to process %d",
+								 (SendStop ? "SIGSTOP" : "SIGQUIT"),
+								 (int) PgArchPID)));
+		signal_child(PgArchPID, (SendStop ? SIGSTOP : SIGQUIT));
+	}
+
 	/*
 	 * Force a power-cycle of the pgarch process too.  (This isn't absolutely
 	 * necessary, but it seems like a good idea for robustness, and it
@@ -3951,6 +3960,7 @@ PostmasterStateMachine(void)
 			Assert(CheckpointerPID == 0);
 			Assert(WalWriterPID == 0);
 			Assert(AutoVacPID == 0);
+			Assert(PgArchPID == 0);
 			/* syslogger is not considered here */
 			pmState = PM_NO_CHILDREN;
 		}
@@ -5230,7 +5240,7 @@ sigusr1_handler(SIGNAL_ARGS)
 		 */
 		Assert(PgArchPID == 0);
 		if (XLogArchivingAlways())
-			PgArchPID = pgarch_start();
+			PgArchPID = StartArchiver();
 
 		/*
 		 * If we aren't planning to enter hot standby mode later, treat
@@ -5275,16 +5285,6 @@ sigusr1_handler(SIGNAL_ARGS)
 	if (StartWorkerNeeded || HaveCrashedWorker)
 		maybe_start_bgworkers();
 
-	if (CheckPostmasterSignal(PMSIGNAL_WAKEN_ARCHIVER) &&
-		PgArchPID != 0)
-	{
-		/*
-		 * Send SIGUSR1 to archiver process, to wake it up and begin archiving
-		 * next WAL file.
-		 */
-		signal_child(PgArchPID, SIGUSR1);
-	}
-
 	/* Tell syslogger to rotate logfile if requested */
 	if (SysLoggerPID != 0)
 	{
@@ -5526,6 +5526,10 @@ StartChildProcess(AuxProcType type)
 				ereport(LOG,
 						(errmsg("could not fork startup process: %m")));
 				break;
+			case ArchiverProcess:
+				ereport(LOG,
+						(errmsg("could not fork archiver process: %m")));
+				break;
 			case BgWriterProcess:
 				ereport(LOG,
 						(errmsg("could not fork background writer process: %m")));
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 88566bd9fa..746bed773e 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -181,6 +181,7 @@ InitProcGlobal(void)
 	ProcGlobal->startupBufferPinWaitBufId = -1;
 	ProcGlobal->walwriterLatch = NULL;
 	ProcGlobal->checkpointerLatch = NULL;
+	ProcGlobal->archiverLatch = NULL;
 	pg_atomic_init_u32(&ProcGlobal->procArrayGroupFirst, INVALID_PGPROCNO);
 	pg_atomic_init_u32(&ProcGlobal->clogGroupFirst, INVALID_PGPROCNO);
 
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 221af87e71..65443063e8 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -339,6 +339,9 @@ extern XLogRecPtr GetRedoRecPtr(void);
 extern XLogRecPtr GetInsertRecPtr(void);
 extern XLogRecPtr GetFlushRecPtr(void);
 extern XLogRecPtr GetLastImportantRecPtr(void);
+extern void XLogArchiveWakeupStart(void);
+extern void XLogArchiveWakeupEnd(void);
+extern void XLogArchiveWakeup(void);
 extern void RemovePromoteSignalFiles(void);
 
 extern bool PromoteIsTriggered(void);
diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h
index 1c67de2ede..54ce0b97d7 100644
--- a/src/include/access/xlogarchive.h
+++ b/src/include/access/xlogarchive.h
@@ -25,6 +25,7 @@ extern void ExecuteRecoveryCommand(const char *command, const char *commandName,
 extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname);
 extern void XLogArchiveNotify(const char *xlog);
 extern void XLogArchiveNotifySeg(XLogSegNo segno);
+extern void XLogArchiveWakeup(void);
 extern void XLogArchiveForceDone(const char *xlog);
 extern bool XLogArchiveCheckDone(const char *xlog);
 extern bool XLogArchiveIsBusy(const char *xlog);
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 72e3352398..bba002ad24 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -418,6 +418,7 @@ typedef enum
 	BootstrapProcess,
 	StartupProcess,
 	BgWriterProcess,
+	ArchiverProcess,
 	CheckpointerProcess,
 	WalWriterProcess,
 	WalReceiverProcess,
@@ -430,6 +431,7 @@ extern AuxProcType MyAuxProcType;
 #define AmBootstrapProcess()		(MyAuxProcType == BootstrapProcess)
 #define AmStartupProcess()			(MyAuxProcType == StartupProcess)
 #define AmBackgroundWriterProcess() (MyAuxProcType == BgWriterProcess)
+#define AmArchiverProcess()			(MyAuxProcType == ArchiverProcess)
 #define AmCheckpointerProcess()		(MyAuxProcType == CheckpointerProcess)
 #define AmWalWriterProcess()		(MyAuxProcType == WalWriterProcess)
 #define AmWalReceiverProcess()		(MyAuxProcType == WalReceiverProcess)
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index b3200874ca..e3ffc63f14 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -32,8 +32,6 @@
  */
 extern int	pgarch_start(void);
 
-#ifdef EXEC_BACKEND
-extern void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
-#endif
+extern void PgArchiverMain(void) pg_attribute_noreturn();
 
 #endif							/* _PGARCH_H */
diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h
index 56c5ec4481..c691acf8cd 100644
--- a/src/include/storage/pmsignal.h
+++ b/src/include/storage/pmsignal.h
@@ -34,7 +34,6 @@ typedef enum
 {
 	PMSIGNAL_RECOVERY_STARTED,	/* recovery has started */
 	PMSIGNAL_BEGIN_HOT_STANDBY, /* begin Hot Standby */
-	PMSIGNAL_WAKEN_ARCHIVER,	/* send a NOTIFY signal to xlog archiver */
 	PMSIGNAL_ROTATE_LOGFILE,	/* send SIGUSR1 to syslogger to rotate logfile */
 	PMSIGNAL_START_AUTOVAC_LAUNCHER,	/* start an autovacuum launcher */
 	PMSIGNAL_START_AUTOVAC_WORKER,	/* start an autovacuum worker */
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 9c9a50ae45..de20520b8c 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -345,6 +345,9 @@ typedef struct PROC_HDR
 	int			startupProcPid;
 	/* Buffer id of the buffer that Startup process waits for pin on, or -1 */
 	int			startupBufferPinWaitBufId;
+	/* Archiver process's latch */
+	Latch	   *archiverLatch;
+	/* Current shared estimate of appropriate spins_per_delay value */
 } PROC_HDR;
 
 extern PGDLLIMPORT PROC_HDR *ProcGlobal;
-- 
2.18.4

----Next_Part(Tue_Oct__6_10_06_44_2020_382)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v39-0004-Shared-memory-based-stats-collector.patch"