From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 10 May 2012 04:54:32 +0000 (-0400)
Subject: Improve tests for postmaster death in auxiliary processes.
X-Git-Tag: REL9_2_BETA1~28
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fd71421b0187de0e2bf76ff66b4a9433bd96c4a0;p=postgresql

Improve tests for postmaster death in auxiliary processes.

In checkpointer and walwriter, avoid calling PostmasterIsAlive unless
WaitLatch has reported WL_POSTMASTER_DEATH.  This saves a kernel call per
iteration of the process's outer loop, which is not all that much, but a
cycle shaved is a cycle earned.  I had already removed the unconditional
PostmasterIsAlive calls in bgwriter and pgstat in previous patches, but
forgot that WL_POSTMASTER_DEATH is supposed to be treated as untrustworthy
(per comment in unix_latch.c); so adjust those two cases to match.

There are a few other places where the same idea might be applied, but only
after substantial code rearrangement, so I didn't bother.
---

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index f72672ef3b..abb665ee50 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -323,9 +323,11 @@ BackgroundWriterMain(void)
 
 		/*
 		 * Emergency bailout if postmaster has died.  This is to avoid the
-		 * necessity for manual cleanup of all postmaster children.
+		 * necessity for manual cleanup of all postmaster children.  Note
+		 * that we mustn't trust the WL_POSTMASTER_DEATH result flag entirely;
+		 * if it is set, recheck with PostmasterIsAlive before believing it.
 		 */
-		if (rc & WL_POSTMASTER_DEATH)
+		if ((rc & WL_POSTMASTER_DEATH) && !PostmasterIsAlive())
 			exit(1);
 
 		prev_hibernate = can_hibernate;
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index fa3435710d..7f8ba95c44 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -374,17 +374,11 @@ CheckpointerMain(void)
 		pg_time_t	now;
 		int			elapsed_secs;
 		int			cur_timeout;
+		int			rc;
 
 		/* Clear any already-pending wakeups */
 		ResetLatch(&MyProc->procLatch);
 
-		/*
-		 * Emergency bailout if postmaster has died.  This is to avoid the
-		 * necessity for manual cleanup of all postmaster children.
-		 */
-		if (!PostmasterIsAlive())
-			exit(1);
-
 		/*
 		 * Process any requests or signals received recently.
 		 */
@@ -581,9 +575,18 @@ CheckpointerMain(void)
 			cur_timeout = Min(cur_timeout, XLogArchiveTimeout - elapsed_secs);
 		}
 
-		(void) WaitLatch(&MyProc->procLatch,
-						 WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
-						 cur_timeout * 1000L /* convert to ms */);
+		rc = WaitLatch(&MyProc->procLatch,
+					   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
+					   cur_timeout * 1000L /* convert to ms */);
+
+		/*
+		 * Emergency bailout if postmaster has died.  This is to avoid the
+		 * necessity for manual cleanup of all postmaster children.  Note
+		 * that we mustn't trust the WL_POSTMASTER_DEATH result flag entirely;
+		 * if it is set, recheck with PostmasterIsAlive before believing it.
+		 */
+		if ((rc & WL_POSTMASTER_DEATH) && !PostmasterIsAlive())
+			exit(1);
 	}
 }
 
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 191813a078..15002dcd9d 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3225,8 +3225,13 @@ PgstatCollectorMain(int argc, char *argv[])
 							   pgStatSock,
 							   -1L);
 
-		/* Check for postmaster death */
-		if (wr & WL_POSTMASTER_DEATH)
+		/*
+		 * Emergency bailout if postmaster has died.  This is to avoid the
+		 * necessity for manual cleanup of all postmaster children.  Note
+		 * that we mustn't trust the WL_POSTMASTER_DEATH result flag entirely;
+		 * if it is set, recheck with PostmasterIsAlive before believing it.
+		 */
+		if ((wr & WL_POSTMASTER_DEATH) && !PostmasterIsAlive())
 			break;
 	}							/* end of outer loop */
 
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 733d01fd5b..886ad4f926 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -246,6 +246,7 @@ WalWriterMain(void)
 	for (;;)
 	{
 		long	cur_timeout;
+		int		rc;
 
 		/*
 		 * Advertise whether we might hibernate in this cycle.  We do this
@@ -265,13 +266,6 @@ WalWriterMain(void)
 		/* Clear any already-pending wakeups */
 		ResetLatch(&MyProc->procLatch);
 
-		/*
-		 * Emergency bailout if postmaster has died.  This is to avoid the
-		 * necessity for manual cleanup of all postmaster children.
-		 */
-		if (!PostmasterIsAlive())
-			exit(1);
-
 		/*
 		 * Process any requests or signals received recently.
 		 */
@@ -305,9 +299,18 @@ WalWriterMain(void)
 		else
 			cur_timeout = WalWriterDelay * HIBERNATE_FACTOR;
 
-		(void) WaitLatch(&MyProc->procLatch,
-						 WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
-						 cur_timeout);
+		rc = WaitLatch(&MyProc->procLatch,
+					   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
+					   cur_timeout);
+
+		/*
+		 * Emergency bailout if postmaster has died.  This is to avoid the
+		 * necessity for manual cleanup of all postmaster children.  Note
+		 * that we mustn't trust the WL_POSTMASTER_DEATH result flag entirely;
+		 * if it is set, recheck with PostmasterIsAlive before believing it.
+		 */
+		if ((rc & WL_POSTMASTER_DEATH) && !PostmasterIsAlive())
+			exit(1);
 	}
 }