revert_remove_dedicated_latch_for_startup_process_v1.patch

text/plain

Filename: revert_remove_dedicated_latch_for_startup_process_v1.patch
Type: text/plain
Part: 0
Message: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module

Patch

Format: unified
Series: patch v1
File+
src/backend/access/transam/xlog.c 25 15
src/backend/postmaster/startup.c 19 5
src/backend/storage/ipc/standby.c 8 1
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 8dd225c2e1..b1e5d2dbff 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -681,8 +681,18 @@ typedef struct XLogCtlData
 	 * recoveryWakeupLatch is used to wake up the startup process to continue
 	 * WAL replay, if it is waiting for WAL to arrive or failover trigger file
 	 * to appear.
+	 *
+	 * Note that the startup process also uses another latch, its procLatch,
+	 * to wait for recovery conflict. If we get rid of recoveryWakeupLatch for
+	 * signaling the startup process in favor of using its procLatch, which
+	 * comports better with possible generic signal handlers using that latch.
+	 * But we should not do that because the startup process doesn't assume
+	 * that it's waken up by walreceiver process or SIGHUP signal handler
+	 * while it's waiting for recovery conflict. The separate latches,
+	 * recoveryWakeupLatch and procLatch, should be used for inter-process
+	 * communication for WAL replay and recovery conflict, respectively.
 	 */
-	Latch		*recoveryWakeupLatch;
+	Latch		recoveryWakeupLatch;
 
 	/*
 	 * During recovery, we keep a copy of the latest checkpoint record here.
@@ -5186,6 +5196,7 @@ XLOGShmemInit(void)
 	SpinLockInit(&XLogCtl->Insert.insertpos_lck);
 	SpinLockInit(&XLogCtl->info_lck);
 	SpinLockInit(&XLogCtl->ulsn_lck);
+	InitSharedLatch(&XLogCtl->recoveryWakeupLatch);
 }
 
 /*
@@ -6121,7 +6132,7 @@ recoveryApplyDelay(XLogReaderState *record)
 
 	while (true)
 	{
-		ResetLatch(MyLatch);
+		ResetLatch(&XLogCtl->recoveryWakeupLatch);
 
 		/* might change the trigger file's location */
 		HandleStartupProcInterrupts();
@@ -6140,7 +6151,7 @@ recoveryApplyDelay(XLogReaderState *record)
 
 		elog(DEBUG2, "recovery apply delay %ld milliseconds", msecs);
 
-		(void) WaitLatch(MyLatch,
+		(void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
 						 WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
 						 msecs,
 						 WAIT_EVENT_RECOVERY_APPLY_DELAY);
@@ -6469,11 +6480,11 @@ StartupXLOG(void)
 	}
 
 	/*
-	 * Advertise our latch that other processes can use to wake us up
-	 * if we're going to sleep during recovery.
+	 * Take ownership of the wakeup latch if we're going to sleep during
+	 * recovery.
 	 */
 	if (ArchiveRecoveryRequested)
-		XLogCtl->recoveryWakeupLatch = &MyProc->procLatch;
+		OwnLatch(&XLogCtl->recoveryWakeupLatch);
 
 	/* Set up XLOG reader facility */
 	MemSet(&private, 0, sizeof(XLogPageReadPrivate));
@@ -7484,11 +7495,11 @@ StartupXLOG(void)
 		ResetUnloggedRelations(UNLOGGED_RELATION_INIT);
 
 	/*
-	 * We don't need the latch anymore. It's not strictly necessary to reset
-	 * it to NULL, but let's do it for the sake of tidiness.
+	 * We don't need the latch anymore. It's not strictly necessary to disown
+	 * it, but let's do it for the sake of tidiness.
 	 */
 	if (ArchiveRecoveryRequested)
-		XLogCtl->recoveryWakeupLatch = NULL;
+		DisownLatch(&XLogCtl->recoveryWakeupLatch);
 
 	/*
 	 * We are now done reading the xlog from stream. Turn off streaming
@@ -12300,12 +12311,12 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 						wait_time = wal_retrieve_retry_interval -
 							TimestampDifferenceMilliseconds(last_fail_time, now);
 
-						(void) WaitLatch(MyLatch,
+						(void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
 										 WL_LATCH_SET | WL_TIMEOUT |
 										 WL_EXIT_ON_PM_DEATH,
 										 wait_time,
 										 WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL);
-						ResetLatch(MyLatch);
+						ResetLatch(&XLogCtl->recoveryWakeupLatch);
 						now = GetCurrentTimestamp();
 
 						/* Handle interrupt signals of startup process */
@@ -12559,11 +12570,11 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 					 * to react to a trigger file promptly and to check if the
 					 * WAL receiver is still active.
 					 */
-					(void) WaitLatch(MyLatch,
+					(void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
 									 WL_LATCH_SET | WL_TIMEOUT |
 									 WL_EXIT_ON_PM_DEATH,
 									 5000L, WAIT_EVENT_RECOVERY_WAL_STREAM);
-					ResetLatch(MyLatch);
+					ResetLatch(&XLogCtl->recoveryWakeupLatch);
 					break;
 				}
 
@@ -12735,8 +12746,7 @@ CheckPromoteSignal(void)
 void
 WakeupRecovery(void)
 {
-	if (XLogCtl->recoveryWakeupLatch)
-		SetLatch(XLogCtl->recoveryWakeupLatch);
+	SetLatch(&XLogCtl->recoveryWakeupLatch);
 }
 
 /*
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index eab9c8c4ed..64af7b8707 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -37,6 +37,7 @@
 /*
  * Flags set by interrupt handlers for later service in the redo loop.
  */
+static volatile sig_atomic_t got_SIGHUP = false;
 static volatile sig_atomic_t shutdown_requested = false;
 static volatile sig_atomic_t promote_signaled = false;
 
@@ -48,6 +49,7 @@ static volatile sig_atomic_t in_restore_command = false;
 
 /* Signal handlers */
 static void StartupProcTriggerHandler(SIGNAL_ARGS);
+static void StartupProcSigHupHandler(SIGNAL_ARGS);
 
 
 /* --------------------------------
@@ -62,7 +64,19 @@ StartupProcTriggerHandler(SIGNAL_ARGS)
 	int			save_errno = errno;
 
 	promote_signaled = true;
-	SetLatch(MyLatch);
+	WakeupRecovery();
+
+	errno = save_errno;
+}
+
+/* SIGHUP: set flag to re-read config file at next convenient time */
+static void
+StartupProcSigHupHandler(SIGNAL_ARGS)
+{
+	int			save_errno = errno;
+
+	got_SIGHUP = true;
+	WakeupRecovery();
 
 	errno = save_errno;
 }
@@ -77,7 +91,7 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
 		proc_exit(1);
 	else
 		shutdown_requested = true;
-	SetLatch(MyLatch);
+	WakeupRecovery();
 
 	errno = save_errno;
 }
@@ -123,9 +137,9 @@ HandleStartupProcInterrupts(void)
 	/*
 	 * Process any requests or signals received recently.
 	 */
-	if (ConfigReloadPending)
+	if (got_SIGHUP)
 	{
-		ConfigReloadPending = false;
+		got_SIGHUP = false;
 		StartupRereadConfig();
 	}
 
@@ -158,7 +172,7 @@ StartupProcessMain(void)
 	/*
 	 * Properly accept or ignore signals the postmaster might send us.
 	 */
-	pqsignal(SIGHUP, SignalHandlerForConfigReload); /* reload config file */
+	pqsignal(SIGHUP, StartupProcSigHupHandler); /* reload config file */
 	pqsignal(SIGINT, SIG_IGN);	/* ignore query cancel */
 	pqsignal(SIGTERM, StartupProcShutdownHandler);	/* request shutdown */
 	/* SIGQUIT handler was already set up by InitPostmasterChild */
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 4ea3cf1f5c..92d9027776 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -519,7 +519,14 @@ ResolveRecoveryConflictWithBufferPin(void)
 		enable_timeouts(timeouts, 2);
 	}
 
-	/* Wait to be signaled by UnpinBuffer() */
+	/*
+	 * Wait to be signaled by UnpinBuffer().
+	 *
+	 * We assume that only UnpinBuffer() and the timeout requests established
+	 * above can wake us up here. WakeupRecovery() called by walreceiver or
+	 * SIGHUP signal handler, etc cannot do that because it uses the different
+	 * latch from that ProcWaitForSignal() waits on.
+	 */
 	ProcWaitForSignal(PG_WAIT_BUFFER_PIN);
 
 	/*