On 08.07.2011 13:58, Florian Pflug wrote:
On Jul8, 2011, at 11:57 , Peter Geoghegan wrote:
On 7 July 2011 19:15, Robert Haas<robertmh...@gmail.com>  wrote:
I'm not concerned about the possibility of spurious extra cycles of
auxiliary process event loops - should I be?

A tight loop would be bad, but an occasional spurious wake-up seems harmless.

We should also assert !PostmasterIsAlive() from within the latch code
after waking due to apparent Postmaster death. The reason that I don't
want to follow Florian's suggestion to check it in production is that
I don't know what to do if the postmaster turns out to be alive. Why
is it more reasonable to try again than to just return?

I'd say return, but don't indicate postmaster death in the return value
if PostmasterIsAlive() returns true. Or don't call PostmasterIsAlive() in
WaitLatch(), and return indicating postmaster death whenever select()
says so, and put the burden of re-checking on the callers.

I put the burden on the callers. Removing the return value from WaitLatch() altogether just makes life unnecessarily difficult for callers that could safely use that information, even if you sometimes get spurious wakeups. In particular, the coding in pgarch.c is nicer if you can simply check the return code for WL_TIMEOUT, rather than call time(NULL) to figure out if the timeout was reached.

Attached is a new version of this patch. PostmasterIsAlive() now uses read() on the pipe instead of kill().

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 9938,9944 **** HandleStartupProcInterrupts(void)
  	 * Emergency bailout if postmaster has died.  This is to avoid the
  	 * necessity for manual cleanup of all postmaster children.
  	 */
! 	if (IsUnderPostmaster && !PostmasterIsAlive(true))
  		exit(1);
  }
  
--- 9938,9944 ----
  	 * Emergency bailout if postmaster has died.  This is to avoid the
  	 * necessity for manual cleanup of all postmaster children.
  	 */
! 	if (IsUnderPostmaster && !PostmasterIsAlive())
  		exit(1);
  }
  
***************
*** 10165,10171 **** retry:
  					/*
  					 * Wait for more WAL to arrive, or timeout to be reached
  					 */
! 					WaitLatch(&XLogCtl->recoveryWakeupLatch, 5000000L);
  					ResetLatch(&XLogCtl->recoveryWakeupLatch);
  				}
  				else
--- 10165,10171 ----
  					/*
  					 * Wait for more WAL to arrive, or timeout to be reached
  					 */
! 					WaitLatch(&XLogCtl->recoveryWakeupLatch, WL_LATCH_SET | WL_TIMEOUT, 5000000L);
  					ResetLatch(&XLogCtl->recoveryWakeupLatch);
  				}
  				else
*** a/src/backend/port/unix_latch.c
--- b/src/backend/port/unix_latch.c
***************
*** 93,98 ****
--- 93,99 ----
  #endif
  
  #include "miscadmin.h"
+ #include "postmaster/postmaster.h"
  #include "storage/latch.h"
  #include "storage/shmem.h"
  
***************
*** 176,209 **** DisownLatch(volatile Latch *latch)
  }
  
  /*
!  * Wait for given latch to be set or until timeout is exceeded.
!  * If the latch is already set, the function returns immediately.
   *
!  * The 'timeout' is given in microseconds, and -1 means wait forever.
!  * On some platforms, signals cause the timeout to be restarted, so beware
!  * that the function can sleep for several times longer than the specified
!  * timeout.
   *
   * The latch must be owned by the current process, ie. it must be a
   * backend-local latch initialized with InitLatch, or a shared latch
   * associated with the current process by calling OwnLatch.
   *
!  * Returns 'true' if the latch was set, or 'false' if timeout was reached.
   */
! bool
! WaitLatch(volatile Latch *latch, long timeout)
  {
! 	return WaitLatchOrSocket(latch, PGINVALID_SOCKET, false, false, timeout) > 0;
  }
  
  /*
!  * Like WaitLatch, but will also return when there's data available in
!  * 'sock' for reading or writing. Returns 0 if timeout was reached,
!  * 1 if the latch was set, 2 if the socket became readable or writable.
   */
  int
! WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
! 				  bool forWrite, long timeout)
  {
  	struct timeval tv,
  			   *tvp = NULL;
--- 177,220 ----
  }
  
  /*
!  * Wait for a given latch to be set, postmaster death, or until timeout is
!  * exceeded. 'wakeEvents' is a bitmask that specifies which of those events
!  * to wait for. If the latch is already set (and WL_LATCH_SET is given), the
!  * function returns immediately.
   *
!  * The 'timeout' is given in microseconds. It must be >= 0 if WL_TIMEOUT
!  * event is given, otherwise it is ignored. On some platforms, signals cause
!  * the timeout to be restarted, so beware that the function can sleep for
!  * several times longer than the specified timeout.
   *
   * The latch must be owned by the current process, ie. it must be a
   * backend-local latch initialized with InitLatch, or a shared latch
   * associated with the current process by calling OwnLatch.
   *
!  * Returns bit field indicating which condition(s) caused the wake-up. Note
!  * that if multiple wake-up conditions are true, there is no guarantee that
!  * we return all of them in one call, but we will return at least one. Also,
!  * according to the select(2) man page on Linux, select(2) may spuriously
!  * return and report a file descriptor as readable, when it's not. We use
!  * select(2), so WaitLatch can also spuriously claim that a socket is
!  * readable, or postmaster has died, even when none of the wake conditions
!  * have been satisfied. That should be rare in practice, but the caller
!  * should not use the return value for anything critical, re-checking the
!  * situation with PostmasterIsAlive() or read() on a socket if necessary.
   */
! int
! WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
  {
! 	return WaitLatchOrSocket(latch, wakeEvents, PGINVALID_SOCKET, timeout);
  }
  
  /*
!  * Like WaitLatch, but with an extra socket argument for WL_SOCKET_*
!  * conditions.
   */
  int
! WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,
! 				  long timeout)
  {
  	struct timeval tv,
  			   *tvp = NULL;
***************
*** 212,230 **** WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
  	int			rc;
  	int			result = 0;
  
! 	if (latch->owner_pid != MyProcPid)
  		elog(ERROR, "cannot wait on a latch owned by another process");
  
  	/* Initialize timeout */
! 	if (timeout >= 0)
  	{
  		tv.tv_sec = timeout / 1000000L;
  		tv.tv_usec = timeout % 1000000L;
  		tvp = &tv;
  	}
  
  	waiting = true;
! 	for (;;)
  	{
  		int			hifd;
  
--- 223,248 ----
  	int			rc;
  	int			result = 0;
  
! 	/* Ignore WL_SOCKET_* events if no valid socket is given */
! 	if (sock == PGINVALID_SOCKET)
! 		wakeEvents &= ~(WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE);
! 
! 	Assert(wakeEvents != 0);	/* must have at least one wake event */
! 
! 	if ((wakeEvents & WL_LATCH_SET) && latch->owner_pid != MyProcPid)
  		elog(ERROR, "cannot wait on a latch owned by another process");
  
  	/* Initialize timeout */
! 	if (wakeEvents & WL_TIMEOUT)
  	{
+ 		Assert(timeout >= 0);
  		tv.tv_sec = timeout / 1000000L;
  		tv.tv_usec = timeout % 1000000L;
  		tvp = &tv;
  	}
  
  	waiting = true;
! 	do
  	{
  		int			hifd;
  
***************
*** 235,250 **** WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
  		 * do that), and the select() will return immediately.
  		 */
  		drainSelfPipe();
! 		if (latch->is_set)
  		{
! 			result = 1;
  			break;
  		}
  
  		FD_ZERO(&input_mask);
  		FD_SET(selfpipe_readfd, &input_mask);
  		hifd = selfpipe_readfd;
! 		if (sock != PGINVALID_SOCKET && forRead)
  		{
  			FD_SET(sock, &input_mask);
  			if (sock > hifd)
--- 253,280 ----
  		 * do that), and the select() will return immediately.
  		 */
  		drainSelfPipe();
! 		if ((wakeEvents & WL_LATCH_SET) && latch->is_set)
  		{
! 			result |= WL_LATCH_SET;
! 			/*
! 			 * Leave loop immediately, avoid blocking again. We don't attempt
! 			 * to report any other events that might also be satisfied.
! 			 */
  			break;
  		}
  
  		FD_ZERO(&input_mask);
  		FD_SET(selfpipe_readfd, &input_mask);
  		hifd = selfpipe_readfd;
! 
! 		if (wakeEvents & WL_POSTMASTER_DEATH)
! 		{
! 			FD_SET(postmaster_alive_fds[POSTMASTER_FD_WATCH], &input_mask);
! 			if (postmaster_alive_fds[POSTMASTER_FD_WATCH] > hifd)
! 				hifd = postmaster_alive_fds[POSTMASTER_FD_WATCH];
! 		}
! 
! 		if (wakeEvents & WL_SOCKET_READABLE)
  		{
  			FD_SET(sock, &input_mask);
  			if (sock > hifd)
***************
*** 252,265 **** WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
  		}
  
  		FD_ZERO(&output_mask);
! 		if (sock != PGINVALID_SOCKET && forWrite)
  		{
  			FD_SET(sock, &output_mask);
  			if (sock > hifd)
  				hifd = sock;
  		}
  
  		rc = select(hifd + 1, &input_mask, &output_mask, NULL, tvp);
  		if (rc < 0)
  		{
  			if (errno == EINTR)
--- 282,298 ----
  		}
  
  		FD_ZERO(&output_mask);
! 		if (wakeEvents & WL_SOCKET_WRITEABLE)
  		{
  			FD_SET(sock, &output_mask);
  			if (sock > hifd)
  				hifd = sock;
  		}
  
+ 		/* Sleep */
  		rc = select(hifd + 1, &input_mask, &output_mask, NULL, tvp);
+ 
+ 		/* Check return code */
  		if (rc < 0)
  		{
  			if (errno == EINTR)
***************
*** 268,287 **** WaitLatchOrSocket(volatile Latch *latch, pgsocket sock, bool forRead,
  					(errcode_for_socket_access(),
  					 errmsg("select() failed: %m")));
  		}
! 		if (rc == 0)
  		{
  			/* timeout exceeded */
! 			result = 0;
! 			break;
  		}
! 		if (sock != PGINVALID_SOCKET &&
! 			((forRead && FD_ISSET(sock, &input_mask)) ||
! 			 (forWrite && FD_ISSET(sock, &output_mask))))
  		{
! 			result = 2;
! 			break;				/* data available in socket */
  		}
! 	}
  	waiting = false;
  
  	return result;
--- 301,326 ----
  					(errcode_for_socket_access(),
  					 errmsg("select() failed: %m")));
  		}
! 		if (rc == 0 && (wakeEvents & WL_TIMEOUT))
  		{
  			/* timeout exceeded */
! 			result |= WL_TIMEOUT;
  		}
! 		if ((wakeEvents & WL_SOCKET_READABLE) && FD_ISSET(sock, &input_mask))
  		{
! 			/* data available in socket */
! 			result |= WL_SOCKET_READABLE;
  		}
! 		if ((wakeEvents & WL_SOCKET_WRITEABLE) && FD_ISSET(sock, &output_mask))
! 		{
! 			result |= WL_SOCKET_WRITEABLE;
! 		}
! 		if ((wakeEvents & WL_POSTMASTER_DEATH) &&
! 			 FD_ISSET(postmaster_alive_fds[POSTMASTER_FD_WATCH], &input_mask))
! 		{
! 			result |= WL_POSTMASTER_DEATH;
! 		}
! 	} while(result == 0);
  	waiting = false;
  
  	return result;
*** a/src/backend/port/win32_latch.c
--- b/src/backend/port/win32_latch.c
***************
*** 23,28 ****
--- 23,29 ----
  #include <unistd.h>
  
  #include "miscadmin.h"
+ #include "postmaster/postmaster.h"
  #include "replication/walsender.h"
  #include "storage/latch.h"
  #include "storage/shmem.h"
***************
*** 81,123 **** DisownLatch(volatile Latch *latch)
  	latch->owner_pid = 0;
  }
  
! bool
! WaitLatch(volatile Latch *latch, long timeout)
  {
! 	return WaitLatchOrSocket(latch, PGINVALID_SOCKET, false, false, timeout) > 0;
  }
  
  int
! WaitLatchOrSocket(volatile Latch *latch, SOCKET sock, bool forRead,
! 				  bool forWrite, long timeout)
  {
  	DWORD		rc;
! 	HANDLE		events[3];
  	HANDLE		latchevent;
! 	HANDLE		sockevent = WSA_INVALID_EVENT;	/* silence compiler */
  	int			numevents;
  	int			result = 0;
  
  	latchevent = latch->event;
  
  	events[0] = latchevent;
  	events[1] = pgwin32_signal_event;
  	numevents = 2;
! 	if (sock != PGINVALID_SOCKET && (forRead || forWrite))
  	{
  		int			flags = 0;
  
! 		if (forRead)
  			flags |= FD_READ;
! 		if (forWrite)
  			flags |= FD_WRITE;
  
  		sockevent = WSACreateEvent();
  		WSAEventSelect(sock, sockevent, flags);
  		events[numevents++] = sockevent;
  	}
  
! 	for (;;)
  	{
  		/*
  		 * Reset the event, and check if the latch is set already. If someone
--- 82,148 ----
  	latch->owner_pid = 0;
  }
  
! int
! WaitLatch(volatile Latch *latch, int wakeEvents, long timeout)
  {
! 	return WaitLatchOrSocket(latch, wakeEvents, PGINVALID_SOCKET, timeout);
  }
  
  int
! WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, SOCKET sock,
! 				  long timeout)
  {
  	DWORD		rc;
! 	HANDLE		events[4];
  	HANDLE		latchevent;
! 	HANDLE		sockevent = WSA_INVALID_EVENT;
  	int			numevents;
  	int			result = 0;
+ 	int			pmdeath_eventno;
+ 	long		timeout_ms;
+ 
+ 	Assert(wakeEvents != 0);
+ 
+ 	/* Ignore WL_SOCKET_* events if no valid socket is given */
+ 	if (sock == PGINVALID_SOCKET)
+ 		wakeEvents &= ~(WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE);
+ 
+ 	/* Convert timeout to milliseconds for WaitForMultipleObjects() */
+ 	if (wakeEvents & WL_TIMEOUT)
+ 	{
+ 		Assert(timeout >= 0);
+ 		timeout_ms = timeout / 1000;
+ 	}
+ 	else
+ 		timeout_ms = INFINITE;
  
+ 	/* Construct an array of event handles for WaitforMultipleObjects() */
  	latchevent = latch->event;
  
  	events[0] = latchevent;
  	events[1] = pgwin32_signal_event;
  	numevents = 2;
! 	if (((wakeEvents & WL_SOCKET_READABLE) ||
! 		 (wakeEvents & WL_SOCKET_WRITEABLE)))
  	{
  		int			flags = 0;
  
! 		if (wakeEvents & WL_SOCKET_READABLE)
  			flags |= FD_READ;
! 		if (wakeEvents & WL_SOCKET_WRITEABLE)
  			flags |= FD_WRITE;
  
  		sockevent = WSACreateEvent();
  		WSAEventSelect(sock, sockevent, flags);
  		events[numevents++] = sockevent;
  	}
+ 	if (wakeEvents & WL_POSTMASTER_DEATH)
+ 	{
+ 		pmdeath_eventno = numevents;
+ 		events[numevents++] = PostmasterHandle;
+ 	}
  
! 	do
  	{
  		/*
  		 * Reset the event, and check if the latch is set already. If someone
***************
*** 127,171 **** WaitLatchOrSocket(volatile Latch *latch, SOCKET sock, bool forRead,
  		 */
  		if (!ResetEvent(latchevent))
  			elog(ERROR, "ResetEvent failed: error code %d", (int) GetLastError());
! 		if (latch->is_set)
  		{
! 			result = 1;
  			break;
  		}
  
! 		rc = WaitForMultipleObjects(numevents, events, FALSE,
! 							   (timeout >= 0) ? (timeout / 1000) : INFINITE);
  		if (rc == WAIT_FAILED)
  			elog(ERROR, "WaitForMultipleObjects() failed: error code %d", (int) GetLastError());
  		else if (rc == WAIT_TIMEOUT)
  		{
! 			result = 0;
! 			break;
  		}
! 		else if (rc == WAIT_OBJECT_0 + 1)
! 			pgwin32_dispatch_queued_signals();
! 		else if (rc == WAIT_OBJECT_0 + 2)
  		{
  			WSANETWORKEVENTS resEvents;
  
- 			Assert(sock != PGINVALID_SOCKET);
- 
  			ZeroMemory(&resEvents, sizeof(resEvents));
  			if (WSAEnumNetworkEvents(sock, sockevent, &resEvents) == SOCKET_ERROR)
  				ereport(FATAL,
  						(errmsg_internal("failed to enumerate network events: %i", (int) GetLastError())));
  
! 			if ((forRead && resEvents.lNetworkEvents & FD_READ) ||
! 				(forWrite && resEvents.lNetworkEvents & FD_WRITE))
! 				result = 2;
! 			break;
  		}
  		else if (rc != WAIT_OBJECT_0)
  			elog(ERROR, "unexpected return code from WaitForMultipleObjects(): %d", (int) rc);
  	}
  
  	/* Clean up the handle we created for the socket */
! 	if (sock != PGINVALID_SOCKET && (forRead || forWrite))
  	{
  		WSAEventSelect(sock, sockevent, 0);
  		WSACloseEvent(sockevent);
--- 152,215 ----
  		 */
  		if (!ResetEvent(latchevent))
  			elog(ERROR, "ResetEvent failed: error code %d", (int) GetLastError());
! 		if (latch->is_set && (wakeEvents & WL_LATCH_SET))
  		{
! 			result |= WL_LATCH_SET;
! 			/*
! 			 * Leave loop immediately, avoid blocking again. We don't attempt
! 			 * to report any other events that might also be satisfied.
! 			 */
  			break;
  		}
  
! 		rc = WaitForMultipleObjects(numevents, events, FALSE, timeout_ms);
! 
  		if (rc == WAIT_FAILED)
  			elog(ERROR, "WaitForMultipleObjects() failed: error code %d", (int) GetLastError());
+ 
+ 		/* Participate in Windows signal emulation */
+ 		else if (rc == WAIT_OBJECT_0 + 1)
+ 			pgwin32_dispatch_queued_signals();
+ 
+ 		else if ((wakeEvents & WL_POSTMASTER_DEATH) &&
+ 			rc == WAIT_OBJECT_0 + pmdeath_eventno)
+ 		{
+ 			/* Postmaster died */
+ 			result |= WL_POSTMASTER_DEATH;
+ 		}
  		else if (rc == WAIT_TIMEOUT)
  		{
! 			result |= WL_TIMEOUT;
  		}
! 		else if ((wakeEvents & (WL_SOCKET_READABLE | WL_SOCKET_WRITEABLE)) != 0 &&
! 				 rc == WAIT_OBJECT_0 + 2)	/* socket is at event slot 2 */
  		{
  			WSANETWORKEVENTS resEvents;
  
  			ZeroMemory(&resEvents, sizeof(resEvents));
  			if (WSAEnumNetworkEvents(sock, sockevent, &resEvents) == SOCKET_ERROR)
  				ereport(FATAL,
  						(errmsg_internal("failed to enumerate network events: %i", (int) GetLastError())));
  
! 			if ((wakeEvents & WL_SOCKET_READABLE) &&
! 				(resEvents.lNetworkEvents & FD_READ))
! 			{
! 				result |= WL_SOCKET_READABLE;
! 			}
! 			if ((wakeEvents & WL_SOCKET_WRITEABLE) &&
! 				(resEvents.lNetworkEvents & FD_WRITE))
! 			{
! 				result |= WL_SOCKET_WRITEABLE;
! 			}
  		}
+ 		/* Otherwise it must be the latch event */
  		else if (rc != WAIT_OBJECT_0)
  			elog(ERROR, "unexpected return code from WaitForMultipleObjects(): %d", (int) rc);
  	}
+ 	while(result == 0);
  
  	/* Clean up the handle we created for the socket */
! 	if (sockevent != WSA_INVALID_EVENT)
  	{
  		WSAEventSelect(sock, sockevent, 0);
  		WSACloseEvent(sockevent);
*** a/src/backend/postmaster/autovacuum.c
--- b/src/backend/postmaster/autovacuum.c
***************
*** 556,562 **** AutoVacLauncherMain(int argc, char *argv[])
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive(true))
  			proc_exit(1);
  
  		launcher_determine_sleep((AutoVacuumShmem->av_freeWorkers != NULL),
--- 556,562 ----
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive())
  			proc_exit(1);
  
  		launcher_determine_sleep((AutoVacuumShmem->av_freeWorkers != NULL),
***************
*** 593,599 **** AutoVacLauncherMain(int argc, char *argv[])
  			 * Emergency bailout if postmaster has died.  This is to avoid the
  			 * necessity for manual cleanup of all postmaster children.
  			 */
! 			if (!PostmasterIsAlive(true))
  				proc_exit(1);
  
  			if (got_SIGTERM || got_SIGHUP || got_SIGUSR2)
--- 593,599 ----
  			 * Emergency bailout if postmaster has died.  This is to avoid the
  			 * necessity for manual cleanup of all postmaster children.
  			 */
! 			if (!PostmasterIsAlive())
  				proc_exit(1);
  
  			if (got_SIGTERM || got_SIGHUP || got_SIGUSR2)
*** a/src/backend/postmaster/bgwriter.c
--- b/src/backend/postmaster/bgwriter.c
***************
*** 381,387 **** BackgroundWriterMain(void)
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive(true))
  			exit(1);
  
  		/*
--- 381,387 ----
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive())
  			exit(1);
  
  		/*
*** a/src/backend/postmaster/pgarch.c
--- b/src/backend/postmaster/pgarch.c
***************
*** 40,45 ****
--- 40,46 ----
  #include "postmaster/postmaster.h"
  #include "storage/fd.h"
  #include "storage/ipc.h"
+ #include "storage/latch.h"
  #include "storage/pg_shmem.h"
  #include "storage/pmsignal.h"
  #include "utils/guc.h"
***************
*** 87,92 **** static volatile sig_atomic_t got_SIGTERM = false;
--- 88,98 ----
  static volatile sig_atomic_t wakened = false;
  static volatile sig_atomic_t ready_to_stop = false;
  
+ /*
+  * Latch used by signal handlers to wake up the sleep in the main loop.
+  */
+ static Latch mainloop_latch;
+ 
  /* ----------
   * Local function forward declarations
   * ----------
***************
*** 228,233 **** PgArchiverMain(int argc, char *argv[])
--- 234,241 ----
  
  	MyProcPid = getpid();		/* reset MyProcPid */
  
+ 	InitLatch(&mainloop_latch); /* initialize latch used in main loop */
+ 
  	MyStartTime = time(NULL);	/* record Start Time for logging */
  
  	/*
***************
*** 282,287 **** ArchSigHupHandler(SIGNAL_ARGS)
--- 290,297 ----
  {
  	/* set flag to re-read config file at next convenient time */
  	got_SIGHUP = true;
+ 	/* let the waiting loop iterate */
+ 	SetLatch(&mainloop_latch);
  }
  
  /* SIGTERM signal handler for archiver process */
***************
*** 295,300 **** ArchSigTermHandler(SIGNAL_ARGS)
--- 305,312 ----
  	 * archive commands.
  	 */
  	got_SIGTERM = true;
+ 	/* let the waiting loop iterate */
+ 	SetLatch(&mainloop_latch);
  }
  
  /* SIGUSR1 signal handler for archiver process */
***************
*** 303,308 **** pgarch_waken(SIGNAL_ARGS)
--- 315,322 ----
  {
  	/* set flag that there is work to be done */
  	wakened = true;
+ 	/* let the waiting loop iterate */
+ 	SetLatch(&mainloop_latch);
  }
  
  /* SIGUSR2 signal handler for archiver process */
***************
*** 311,316 **** pgarch_waken_stop(SIGNAL_ARGS)
--- 325,332 ----
  {
  	/* set flag to do a final cycle and shut down afterwards */
  	ready_to_stop = true;
+ 	/* let the waiting loop iterate */
+ 	SetLatch(&mainloop_latch);
  }
  
  /*
***************
*** 321,327 **** pgarch_waken_stop(SIGNAL_ARGS)
  static void
  pgarch_MainLoop(void)
  {
! 	time_t		last_copy_time = 0;
  	bool		time_to_stop;
  
  	/*
--- 337,343 ----
  static void
  pgarch_MainLoop(void)
  {
! 	pg_time_t	last_copy_time = 0;
  	bool		time_to_stop;
  
  	/*
***************
*** 332,339 **** pgarch_MainLoop(void)
--- 348,362 ----
  	 */
  	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 wakes up occasionally to allow herself to be proactive.
+ 	 */
  	do
  	{
+ 		ResetLatch(&mainloop_latch);
+ 
  		/* When we get SIGUSR2, we do one more archive cycle, then exit */
  		time_to_stop = ready_to_stop;
  
***************
*** 371,394 **** pgarch_MainLoop(void)
  		}
  
  		/*
! 		 * 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 wakes up occasionally to allow herself to be proactive.
! 		 *
! 		 * On some platforms, signals won't interrupt the sleep.  To ensure we
! 		 * respond reasonably promptly when someone signals us, break down the
! 		 * sleep into 1-second increments, and check for interrupts after each
! 		 * nap.
  		 */
! 		while (!(wakened || ready_to_stop || got_SIGHUP ||
! 				 !PostmasterIsAlive(true)))
  		{
! 			time_t		curtime;
  
! 			pg_usleep(1000000L);
! 			curtime = time(NULL);
! 			if ((unsigned int) (curtime - last_copy_time) >=
! 				(unsigned int) PGARCH_AUTOWAKE_INTERVAL)
  				wakened = true;
  		}
  
--- 394,419 ----
  		}
  
  		/*
! 		 * Sleep until a signal is received, or until a poll is forced by
! 		 ' PGARCH_AUTOWAKE_INTERVAL having passed since last_copy_time, or
! 		 * until postmaster dies.
  		 */
! 		if (!time_to_stop) /* Don't wait during last iteration */
  		{
! 			pg_time_t curtime = (pg_time_t) time(NULL);
! 			int		timeout;
  
! 			timeout = PGARCH_AUTOWAKE_INTERVAL - (curtime - last_copy_time);
! 			if (timeout > 0)
! 			{
! 				int rc;
! 				rc = WaitLatch(&mainloop_latch,
! 							   WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
! 							   timeout * 1000000L);
! 				if (rc & WL_TIMEOUT)
! 					wakened = true;
! 			}
! 			else
  				wakened = true;
  		}
  
***************
*** 397,403 **** pgarch_MainLoop(void)
  		 * or after completing one more archiving cycle after receiving
  		 * SIGUSR2.
  		 */
! 	} while (PostmasterIsAlive(true) && !time_to_stop);
  }
  
  /*
--- 422,428 ----
  		 * or after completing one more archiving cycle after receiving
  		 * SIGUSR2.
  		 */
! 	} while (PostmasterIsAlive() && !time_to_stop);
  }
  
  /*
***************
*** 429,435 **** pgarch_ArchiverCopyLoop(void)
  			 * command, and the second is to avoid conflicts with another
  			 * archiver spawned by a newer postmaster.
  			 */
! 			if (got_SIGTERM || !PostmasterIsAlive(true))
  				return;
  
  			/*
--- 454,460 ----
  			 * command, and the second is to avoid conflicts with another
  			 * archiver spawned by a newer postmaster.
  			 */
! 			if (got_SIGTERM || !PostmasterIsAlive())
  				return;
  
  			/*
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***************
*** 3111,3117 **** PgstatCollectorMain(int argc, char *argv[])
  			 * We can only get here if the select/poll timeout elapsed. Check
  			 * for postmaster death.
  			 */
! 			if (!PostmasterIsAlive(true))
  				break;
  		}
  	}							/* end of message-processing loop */
--- 3111,3117 ----
  			 * We can only get here if the select/poll timeout elapsed. Check
  			 * for postmaster death.
  			 */
! 			if (!PostmasterIsAlive())
  				break;
  		}
  	}							/* end of message-processing loop */
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
***************
*** 368,373 **** static int	CountChildren(int target);
--- 368,374 ----
  static bool CreateOptsFile(int argc, char *argv[], char *fullprogname);
  static pid_t StartChildProcess(AuxProcType type);
  static void StartAutovacuumWorker(void);
+ static void InitPostmasterDeathWatchHandle(void);
  
  #ifdef EXEC_BACKEND
  
***************
*** 383,390 **** typedef struct
  	HANDLE		procHandle;
  	DWORD		procId;
  } win32_deadchild_waitinfo;
- 
- HANDLE		PostmasterHandle;
  #endif
  
  static pid_t backend_forkexec(Port *port);
--- 384,389 ----
***************
*** 439,444 **** typedef struct
--- 438,444 ----
  	HANDLE		initial_signal_pipe;
  	HANDLE		syslogPipe[2];
  #else
+ 	int			postmaster_alive_fds[2];
  	int			syslogPipe[2];
  #endif
  	char		my_exec_path[MAXPGPATH];
***************
*** 469,474 **** static void ShmemBackendArrayRemove(Backend *bn);
--- 469,484 ----
  #define EXIT_STATUS_0(st)  ((st) == 0)
  #define EXIT_STATUS_1(st)  (WIFEXITED(st) && WEXITSTATUS(st) == 1)
  
+ #ifndef WIN32
+ /*
+  * File descriptors for pipe used to monitor if postmaster is alive.
+  * First is POSTMASTER_FD_WATCH, second is POSTMASTER_FD_OWN.
+  */
+ int postmaster_alive_fds[2] = { -1, -1 };
+ #else
+ /* Process handle of postmaster used for the same purpose on Windows */
+ HANDLE		PostmasterHandle;
+ #endif
  
  /*
   * Postmaster main entry point
***************
*** 962,969 **** PostmasterMain(int argc, char *argv[])
  	 */
  	BackendList = DLNewList();
  
! #ifdef WIN32
  
  	/*
  	 * Initialize I/O completion port used to deliver list of dead children.
  	 */
--- 972,984 ----
  	 */
  	BackendList = DLNewList();
  
! 	/*
! 	 * Initialize pipe (or process handle on Windows) that allows children to
! 	 * wake up from sleep on postmaster death.
! 	 */
! 	InitPostmasterDeathWatchHandle();
  
+ #ifdef WIN32
  	/*
  	 * Initialize I/O completion port used to deliver list of dead children.
  	 */
***************
*** 971,991 **** PostmasterMain(int argc, char *argv[])
  	if (win32ChildQueue == NULL)
  		ereport(FATAL,
  		   (errmsg("could not create I/O completion port for child queue")));
- 
- 	/*
- 	 * Set up a handle that child processes can use to check whether the
- 	 * postmaster is still running.
- 	 */
- 	if (DuplicateHandle(GetCurrentProcess(),
- 						GetCurrentProcess(),
- 						GetCurrentProcess(),
- 						&PostmasterHandle,
- 						0,
- 						TRUE,
- 						DUPLICATE_SAME_ACCESS) == 0)
- 		ereport(FATAL,
- 				(errmsg_internal("could not duplicate postmaster handle: error code %d",
- 								 (int) GetLastError())));
  #endif
  
  	/*
--- 986,991 ----
***************
*** 1965,1970 **** ClosePostmasterPorts(bool am_syslogger)
--- 1965,1983 ----
  {
  	int			i;
  
+ #ifndef WIN32
+ 	/*
+ 	 * Close the write end of postmaster death watch pipe. It's important to
+ 	 * do this as early as possible, so that if postmaster dies, others won't
+ 	 * think that it's still running because we're holding the pipe open.
+ 	 */
+ 	if (close(postmaster_alive_fds[POSTMASTER_FD_OWN]))
+ 		ereport(FATAL,
+ 			(errcode_for_file_access(),
+ 			 errmsg_internal("could not close postmaster death monitoring pipe in child process: %m")));
+ 	postmaster_alive_fds[POSTMASTER_FD_OWN] = -1;
+ #endif
+ 
  	/* Close the listen sockets */
  	for (i = 0; i < MAXLISTEN; i++)
  	{
***************
*** 4643,4648 **** save_backend_variables(BackendParameters *param, Port *port,
--- 4656,4664 ----
  								 pgwin32_create_signal_listener(childPid),
  								 childProcess))
  		return false;
+ #else
+ 	memcpy(&param->postmaster_alive_fds, &postmaster_alive_fds,
+ 		   sizeof(postmaster_alive_fds));
  #endif
  
  	memcpy(&param->syslogPipe, &syslogPipe, sizeof(syslogPipe));
***************
*** 4858,4863 **** restore_backend_variables(BackendParameters *param, Port *port)
--- 4874,4882 ----
  #ifdef WIN32
  	PostmasterHandle = param->PostmasterHandle;
  	pgwin32_initial_signal_pipe = param->initial_signal_pipe;
+ #else
+ 	memcpy(&postmaster_alive_fds, &param->postmaster_alive_fds,
+ 		   sizeof(postmaster_alive_fds));
  #endif
  
  	memcpy(&syslogPipe, &param->syslogPipe, sizeof(syslogPipe));
***************
*** 4979,4981 **** pgwin32_deadchild_callback(PVOID lpParameter, BOOLEAN TimerOrWaitFired)
--- 4998,5051 ----
  }
  
  #endif   /* WIN32 */
+ 
+ /*
+  * Initialize one and only handle for monitoring postmaster death.
+  *
+  * Called once in the postmaster, so that child processes can subsequently
+  * monitor if their parent is dead.
+  */
+ static void
+ InitPostmasterDeathWatchHandle(void)
+ {
+ #ifndef WIN32
+ 	/*
+ 	 * Create a pipe. Postmaster holds the write end of the pipe open
+ 	 * (POSTMASTER_FD_OWN), and children hold the read end. Children can
+ 	 * pass the read file descriptor to select() to wake up in case postmaster
+ 	 * dies, or check for postmaster death with a (read() == 0). Children must
+ 	 * close the write end as soon as possible after forking, because EOF
+ 	 * won't be signaled in the read end until all processes have closed the
+ 	 * write fd. That is taken care of in ClosePostmasterPorts().
+ 	 */
+ 	Assert(MyProcPid == PostmasterPid);
+ 	if (pipe(postmaster_alive_fds))
+ 		ereport(FATAL,
+ 				(errcode_for_file_access(),
+ 				 errmsg_internal("could not create pipe to monitor postmaster death: %m")));
+ 
+ 	/*
+ 	 * Set O_NONBLOCK to allow testing for the fd's presence with a read()
+ 	 * call.
+ 	 */
+ 	if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH], F_SETFL, O_NONBLOCK))
+ 		ereport(FATAL,
+ 				(errcode_for_socket_access(),
+ 				 errmsg_internal("could not set postmaster death monitoring pipe to non-blocking mode: %m")));
+ 
+ #else
+ 	/*
+ 	 * On Windows, we use a process handle for the same purpose.
+ 	 */
+ 	if (DuplicateHandle(GetCurrentProcess(),
+ 						GetCurrentProcess(),
+ 						GetCurrentProcess(),
+ 						&PostmasterHandle,
+ 						0,
+ 						TRUE,
+ 						DUPLICATE_SAME_ACCESS) == 0)
+ 		ereport(FATAL,
+ 				(errmsg_internal("could not duplicate postmaster handle: error code %d",
+ 								 (int) GetLastError())));
+ #endif   /* WIN32 */
+ }
*** a/src/backend/postmaster/walwriter.c
--- b/src/backend/postmaster/walwriter.c
***************
*** 227,233 **** WalWriterMain(void)
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive(true))
  			exit(1);
  
  		/*
--- 227,233 ----
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive())
  			exit(1);
  
  		/*
*** a/src/backend/replication/syncrep.c
--- b/src/backend/replication/syncrep.c
***************
*** 171,177 **** SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
  		 * postmaster death regularly while waiting. Note that timeout here
  		 * does not necessarily release from loop.
  		 */
! 		WaitLatch(&MyProc->waitLatch, 60000000L);
  
  		/* Must reset the latch before testing state. */
  		ResetLatch(&MyProc->waitLatch);
--- 171,177 ----
  		 * postmaster death regularly while waiting. Note that timeout here
  		 * does not necessarily release from loop.
  		 */
! 		WaitLatch(&MyProc->waitLatch, WL_LATCH_SET | WL_TIMEOUT, 60000000L);
  
  		/* Must reset the latch before testing state. */
  		ResetLatch(&MyProc->waitLatch);
***************
*** 239,245 **** SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
  		 * acknowledgement, because all the wal sender processes will exit. So
  		 * just bail out.
  		 */
! 		if (!PostmasterIsAlive(true))
  		{
  			ProcDiePending = true;
  			whereToSendOutput = DestNone;
--- 239,245 ----
  		 * acknowledgement, because all the wal sender processes will exit. So
  		 * just bail out.
  		 */
! 		if (!PostmasterIsAlive())
  		{
  			ProcDiePending = true;
  			whereToSendOutput = DestNone;
*** a/src/backend/replication/walreceiver.c
--- b/src/backend/replication/walreceiver.c
***************
*** 287,293 **** WalReceiverMain(void)
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive(true))
  			exit(1);
  
  		/*
--- 287,293 ----
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive())
  			exit(1);
  
  		/*
*** a/src/backend/replication/walsender.c
--- b/src/backend/replication/walsender.c
***************
*** 212,218 **** WalSndHandshake(void)
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive(true))
  			exit(1);
  
  		/*
--- 212,218 ----
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive())
  			exit(1);
  
  		/*
***************
*** 713,719 **** WalSndLoop(void)
  		 * Emergency bailout if postmaster has died.  This is to avoid the
  		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive(true))
  			exit(1);
  
  		/* Process any requests or signals received recently */
--- 713,719 ----
  		 * 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 */
***************
*** 779,784 **** WalSndLoop(void)
--- 779,785 ----
  		{
  			TimestampTz finish_time = 0;
  			long		sleeptime;
+ 			int			wakeEvents;
  
  			/* Reschedule replication timeout */
  			if (replication_timeout > 0)
***************
*** 805,813 **** WalSndLoop(void)
  			}
  
  			/* Sleep */
! 			WaitLatchOrSocket(&MyWalSnd->latch, MyProcPort->sock,
! 							  true, pq_is_send_pending(),
! 							  sleeptime * 1000L);
  
  			/* Check for replication timeout */
  			if (replication_timeout > 0 &&
--- 806,816 ----
  			}
  
  			/* Sleep */
! 			wakeEvents  = WL_LATCH_SET | WL_SOCKET_READABLE | WL_TIMEOUT;
! 			if (pq_is_send_pending())
! 				wakeEvents |= WL_SOCKET_WRITEABLE;
! 			WaitLatchOrSocket(&MyWalSnd->latch, wakeEvents,
! 							  MyProcPort->sock, sleeptime * 1000L);
  
  			/* Check for replication timeout */
  			if (replication_timeout > 0 &&
*** a/src/backend/storage/ipc/pmsignal.c
--- b/src/backend/storage/ipc/pmsignal.c
***************
*** 267,308 **** MarkPostmasterChildInactive(void)
  
  /*
   * PostmasterIsAlive - check whether postmaster process is still alive
-  *
-  * amDirectChild should be passed as "true" by code that knows it is
-  * executing in a direct child process of the postmaster; pass "false"
-  * if an indirect child or not sure.  The "true" case uses a faster and
-  * more reliable test, so use it when possible.
   */
  bool
! PostmasterIsAlive(bool amDirectChild)
  {
  #ifndef WIN32
! 	if (amDirectChild)
! 	{
! 		pid_t		ppid = getppid();
  
! 		/* If the postmaster is still our parent, it must be alive. */
! 		if (ppid == PostmasterPid)
  			return true;
! 
! 		/* If the init process is our parent, postmaster must be dead. */
! 		if (ppid == 1)
! 			return false;
! 
! 		/*
! 		 * If we get here, our parent process is neither the postmaster nor
! 		 * init.  This can occur on BSD and MacOS systems if a debugger has
! 		 * been attached.  We fall through to the less-reliable kill() method.
! 		 */
  	}
  
- 	/*
- 	 * Use kill() to see if the postmaster is still alive.	This can sometimes
- 	 * give a false positive result, since the postmaster's PID may get
- 	 * recycled, but it is good enough for existing uses by indirect children
- 	 * and in debugging environments.
- 	 */
- 	return (kill(PostmasterPid, 0) == 0);
  #else							/* WIN32 */
  	return (WaitForSingleObject(PostmasterHandle, 0) == WAIT_TIMEOUT);
  #endif   /* WIN32 */
--- 267,293 ----
  
  /*
   * PostmasterIsAlive - check whether postmaster process is still alive
   */
  bool
! PostmasterIsAlive(void)
  {
  #ifndef WIN32
! 	char c;
! 	ssize_t rc;
  
! 	rc = read(postmaster_alive_fds[POSTMASTER_FD_WATCH], &c, 1);
! 	if (rc < 0)
! 	{
! 		if (errno == EAGAIN || errno == EWOULDBLOCK)
  			return true;
! 		else
! 			elog(FATAL, "read on postmaster death monitoring pipe failed: %m");
  	}
+ 	else if (rc > 0)
+ 		elog(FATAL, "unexpected data in postmaster death monitoring pipe");
+ 
+ 	return false;
  
  #else							/* WIN32 */
  	return (WaitForSingleObject(PostmasterHandle, 0) == WAIT_TIMEOUT);
  #endif   /* WIN32 */
*** a/src/include/postmaster/postmaster.h
--- b/src/include/postmaster/postmaster.h
***************
*** 32,37 **** extern bool restart_after_crash;
--- 32,45 ----
  
  #ifdef WIN32
  extern HANDLE PostmasterHandle;
+ #else
+ extern int postmaster_alive_fds[2];
+ /*
+  * Constants that represent which of postmaster_alive_fds is held by
+  * postmaster, and which is used in children to check for postmaster death.
+  */
+ #define POSTMASTER_FD_WATCH		0	/* used in children to check for postmaster death */
+ #define POSTMASTER_FD_OWN		1	/* kept open by postmaster only */
  #endif
  
  extern const char *progname;
*** a/src/include/storage/latch.h
--- b/src/include/storage/latch.h
***************
*** 31,36 **** typedef struct
--- 31,43 ----
  #endif
  } Latch;
  
+ /* Bitmasks for events that may wake-up WaitLatch() clients */
+ #define WL_LATCH_SET         (1 << 0)
+ #define WL_SOCKET_READABLE   (1 << 1)
+ #define WL_SOCKET_WRITEABLE  (1 << 2)
+ #define WL_TIMEOUT           (1 << 3)
+ #define WL_POSTMASTER_DEATH  (1 << 4)
+ 
  /*
   * prototypes for functions in latch.c
   */
***************
*** 38,46 **** extern void InitLatch(volatile Latch *latch);
  extern void InitSharedLatch(volatile Latch *latch);
  extern void OwnLatch(volatile Latch *latch);
  extern void DisownLatch(volatile Latch *latch);
! extern bool WaitLatch(volatile Latch *latch, long timeout);
! extern int WaitLatchOrSocket(volatile Latch *latch, pgsocket sock,
! 				  bool forRead, bool forWrite, long timeout);
  extern void SetLatch(volatile Latch *latch);
  extern void ResetLatch(volatile Latch *latch);
  
--- 45,53 ----
  extern void InitSharedLatch(volatile Latch *latch);
  extern void OwnLatch(volatile Latch *latch);
  extern void DisownLatch(volatile Latch *latch);
! extern int WaitLatch(volatile Latch *latch, int wakeEvents, long timeout);
! extern int WaitLatchOrSocket(volatile Latch *latch, int wakeEvents,
! 				  pgsocket sock, long timeout);
  extern void SetLatch(volatile Latch *latch);
  extern void ResetLatch(volatile Latch *latch);
  
*** a/src/include/storage/pmsignal.h
--- b/src/include/storage/pmsignal.h
***************
*** 50,55 **** extern bool IsPostmasterChildWalSender(int slot);
  extern void MarkPostmasterChildActive(void);
  extern void MarkPostmasterChildInactive(void);
  extern void MarkPostmasterChildWalSender(void);
! extern bool PostmasterIsAlive(bool amDirectChild);
  
  #endif   /* PMSIGNAL_H */
--- 50,55 ----
  extern void MarkPostmasterChildActive(void);
  extern void MarkPostmasterChildInactive(void);
  extern void MarkPostmasterChildWalSender(void);
! extern bool PostmasterIsAlive(void);
  
  #endif   /* PMSIGNAL_H */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to