Revert SIGUSR1 multiplexing patch, per Tom's objection.

src/backend/access/transam/twophase.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *		$PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.49 2008/12/09 14:28:20 heikki Exp $
+ *		$PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.50 2008/12/09 15:59:39 heikki Exp $
  *
  * NOTES
  *		Each global transaction is associated with a global transaction
@@ -287,7 +287,6 @@ MarkAsPreparing(TransactionId xid, const char *gid,
 	gxact->proc.databaseId = databaseid;
 	gxact->proc.roleId = owner;
 	gxact->proc.inCommit = false;
-	MemSet(gxact->proc.signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
 	gxact->proc.vacuumFlags = 0;
 	gxact->proc.lwWaiting = false;
 	gxact->proc.lwExclusive = false;

src/backend/commands/async.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.143 2008/12/09 14:28:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.144 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -915,10 +915,9 @@ EnableNotifyInterrupt(void)
  *		a frontend command.  Signal handler execution of inbound notifies
  *		is disabled until the next EnableNotifyInterrupt call.
  *
- *		This also needs to be called when SIGUSR1 with 
+ *		The SIGUSR1 signal handler also needs to call this, so as to
- *		PROCSIG_CATCHUP_INTERRUPT is received, so as to prevent conflicts 
+ *		prevent conflicts if one signal interrupts the other.  So we
- *		if one signal interrupts the other.  So we must return the previous 
+ *		must return the previous state of the flag.
- *		state of the flag.
  */
 bool
 DisableNotifyInterrupt(void)
@@ -955,7 +954,7 @@ ProcessIncomingNotify(void)
 				nulls[Natts_pg_listener];
 	bool		catchup_enabled;
 
-	/* Must prevent catchup interrupt while I am running */
+	/* Must prevent SIGUSR1 interrupt while I am running */
 	catchup_enabled = DisableCatchupInterrupt();
 
 	if (Trace_notify)

src/backend/postmaster/autovacuum.c

@@ -55,7 +55,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.89 2008/12/09 14:28:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.90 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1477,7 +1477,7 @@ AutoVacWorkerMain(int argc, char *argv[])
 	pqsignal(SIGALRM, handle_sig_alarm);
 
 	pqsignal(SIGPIPE, SIG_IGN);
-	pqsignal(SIGUSR1, proc_sigusr1_handler);
+	pqsignal(SIGUSR1, CatchupInterruptHandler);
 	/* We don't listen for async notifies */
 	pqsignal(SIGUSR2, SIG_IGN);
 	pqsignal(SIGFPE, FloatExceptionHandler);

src/backend/storage/ipc/sinval.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/sinval.c,v 1.87 2008/12/09 14:28:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/sinval.c,v 1.88 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -26,8 +26,8 @@
  * Because backends sitting idle will not be reading sinval events, we
  * need a way to give an idle backend a swift kick in the rear and make
  * it catch up before the sinval queue overflows and forces it to go
- * through a cache reset exercise.	This is done by sending
+ * through a cache reset exercise.	This is done by sending SIGUSR1
- * PROCSIG_CATCHUP_INTERRUPT to any backend that gets too far behind.
+ * to any backend that gets too far behind.
  *
  * State for catchup events consists of two flags: one saying whether
  * the signal handler is currently allowed to call ProcessCatchupEvent
@@ -144,9 +144,9 @@ ReceiveSharedInvalidMessages(
 
 
 /*
- * HandleCatchupInterrupt
+ * CatchupInterruptHandler
  *
- * This is called when PROCSIG_CATCHUP_INTERRUPT signal is received.
+ * This is the signal handler for SIGUSR1.
  *
  * If we are idle (catchupInterruptEnabled is set), we can safely
  * invoke ProcessCatchupEvent directly.  Otherwise, just set a flag
@@ -156,11 +156,13 @@ ReceiveSharedInvalidMessages(
  * since there's no longer any reason to do anything.)
  */
 void
-HandleCatchupInterrupt(void)
+CatchupInterruptHandler(SIGNAL_ARGS)
 {
+	int			save_errno = errno;
+
 	/*
-	 * Note: this is called by a SIGNAL HANDLER.	
+	 * Note: this is a SIGNAL HANDLER.	You must be very wary what you do
-	 * You must be very wary what you do here.
+	 * here.
 	 */
 
 	/* Don't joggle the elbow of proc_exit */
@@ -214,6 +216,8 @@ HandleCatchupInterrupt(void)
 		 */
 		catchupInterruptOccurred = 1;
 	}
+
+	errno = save_errno;
 }
 
 /*
@@ -285,8 +289,7 @@ DisableCatchupInterrupt(void)
 /*
  * ProcessCatchupEvent
  *
- * Respond to a catchup event (PROCSIG_CATCHUP_INTERRUPT) from another
+ * Respond to a catchup event (SIGUSR1) from another backend.
- * backend.
  *
  * This is called either directly from the SIGUSR1 signal handler,
  * or the next time control reaches the outer idle loop (assuming

src/backend/storage/ipc/sinvaladt.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/sinvaladt.c,v 1.75 2008/12/09 14:28:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/sinvaladt.c,v 1.76 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,7 +21,6 @@
 #include "storage/backendid.h"
 #include "storage/ipc.h"
 #include "storage/proc.h"
-#include "storage/procarray.h"
 #include "storage/shmem.h"
 #include "storage/sinvaladt.h"
 #include "storage/spin.h"
@@ -137,9 +136,9 @@
 /* Per-backend state in shared invalidation structure */
 typedef struct ProcState
 {
-	/* proc is NULL in an inactive ProcState array entry. */
+	/* procPid is zero in an inactive ProcState array entry. */
-	PGPROC	   *proc;			/* PGPROC entry of backend, for signaling */
+	pid_t		procPid;		/* PID of backend, for signaling */
-	/* nextMsgNum is meaningless if proc == NULL or resetState is true. */
+	/* nextMsgNum is meaningless if procPid == 0 or resetState is true. */
 	int			nextMsgNum;		/* next message number to read */
 	bool		resetState;		/* backend needs to reset its state */
 	bool		signaled;		/* backend has been sent catchup signal */
@@ -236,7 +235,7 @@ CreateSharedInvalidationState(void)
 	/* Mark all backends inactive, and initialize nextLXID */
 	for (i = 0; i < shmInvalBuffer->maxBackends; i++)
 	{
-		shmInvalBuffer->procState[i].proc = NULL;			/* inactive */
+		shmInvalBuffer->procState[i].procPid = 0;			/* inactive */
 		shmInvalBuffer->procState[i].nextMsgNum = 0;		/* meaningless */
 		shmInvalBuffer->procState[i].resetState = false;
 		shmInvalBuffer->procState[i].signaled = false;
@@ -267,7 +266,7 @@ SharedInvalBackendInit(void)
 	/* Look for a free entry in the procState array */
 	for (index = 0; index < segP->lastBackend; index++)
 	{
-		if (segP->procState[index].proc == NULL)		/* inactive slot? */
+		if (segP->procState[index].procPid == 0)		/* inactive slot? */
 		{
 			stateP = &segP->procState[index];
 			break;
@@ -279,7 +278,7 @@ SharedInvalBackendInit(void)
 		if (segP->lastBackend < segP->maxBackends)
 		{
 			stateP = &segP->procState[segP->lastBackend];
-			Assert(stateP->proc == NULL);
+			Assert(stateP->procPid == 0);
 			segP->lastBackend++;
 		}
 		else
@@ -304,7 +303,7 @@ SharedInvalBackendInit(void)
 	nextLocalTransactionId = stateP->nextLXID;
 
 	/* mark myself active, with all extant messages already read */
-	stateP->proc = MyProc;
+	stateP->procPid = MyProcPid;
 	stateP->nextMsgNum = segP->maxMsgNum;
 	stateP->resetState = false;
 	stateP->signaled = false;
@@ -342,7 +341,7 @@ CleanupInvalidationState(int status, Datum arg)
 	stateP->nextLXID = nextLocalTransactionId;
 
 	/* Mark myself inactive */
-	stateP->proc = NULL;
+	stateP->procPid = 0;
 	stateP->nextMsgNum = 0;
 	stateP->resetState = false;
 	stateP->signaled = false;
@@ -350,7 +349,7 @@ CleanupInvalidationState(int status, Datum arg)
 	/* Recompute index of last active backend */
 	for (i = segP->lastBackend; i > 0; i--)
 	{
-		if (segP->procState[i - 1].proc != NULL)
+		if (segP->procState[i - 1].procPid != 0)
 			break;
 	}
 	segP->lastBackend = i;
@@ -375,7 +374,7 @@ BackendIdIsActive(int backendID)
 	{
 		ProcState  *stateP = &segP->procState[backendID - 1];
 
-		result = (stateP->proc != NULL);
+		result = (stateP->procPid != 0);
 	}
 	else
 		result = false;
@@ -591,7 +590,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree)
 		int		n = stateP->nextMsgNum;
 
 		/* Ignore if inactive or already in reset state */
-		if (stateP->proc == NULL || stateP->resetState)
+		if (stateP->procPid == 0 || stateP->resetState)
 			continue;
 
 		/*
@@ -645,20 +644,18 @@ SICleanupQueue(bool callerHasWriteLock, int minFree)
 		segP->nextThreshold = (numMsgs / CLEANUP_QUANTUM + 1) * CLEANUP_QUANTUM;
 
 	/*
-	 * Lastly, signal anyone who needs a catchup interrupt.  Since
+	 * Lastly, signal anyone who needs a catchup interrupt.  Since kill()
-	 * SendProcSignal() might not be fast, we don't want to hold locks while
+	 * might not be fast, we don't want to hold locks while executing it.
-	 * executing it.
 	 */
 	if (needSig)
 	{
-		PGPROC *his_proc = needSig->proc;
+		pid_t	his_pid = needSig->procPid;
 
 		needSig->signaled = true;
 		LWLockRelease(SInvalReadLock);
 		LWLockRelease(SInvalWriteLock);
-		elog(DEBUG4, "sending sinval catchup signal to PID %d",
+		elog(DEBUG4, "sending sinval catchup signal to PID %d", (int) his_pid);
-			 (int) his_proc->pid);
+		kill(his_pid, SIGUSR1);
-		SendProcSignal(his_proc, PROCSIG_CATCHUP_INTERRUPT);
 		if (callerHasWriteLock)
 			LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE);
 	}

src/backend/storage/lmgr/proc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.203 2008/12/09 14:28:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.204 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -289,7 +289,6 @@ InitProcess(void)
 	MyProc->databaseId = InvalidOid;
 	MyProc->roleId = InvalidOid;
 	MyProc->inCommit = false;
-	MemSet(MyProc->signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
 	MyProc->vacuumFlags = 0;
 	if (IsAutoVacuumWorkerProcess())
 		MyProc->vacuumFlags |= PROC_IS_AUTOVACUUM;
@@ -429,7 +428,6 @@ InitAuxiliaryProcess(void)
 	MyProc->databaseId = InvalidOid;
 	MyProc->roleId = InvalidOid;
 	MyProc->inCommit = false;
-	MemSet(MyProc->signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
 	/* we don't set the "is autovacuum" flag in the launcher */
 	MyProc->vacuumFlags = 0;
 	MyProc->lwWaiting = false;
@@ -1279,54 +1277,6 @@ ProcSendSignal(int pid)
 		PGSemaphoreUnlock(&proc->sem);
 }
 
-/*
- * SendProcSignal - send the signal with the reason to a process.
- *
- * The process can be a backend or an auxiliary process that has a PGPROC
- * entry, like an autovacuum worker.
- */
-void
-SendProcSignal(PGPROC *proc, ProcSignalReason reason)
-{
-	pid_t pid;
-
-	/*
-	 * If the process is gone, do nothing.
-	 *
-	 * Since there's no locking, it's possible that the process detaches
-	 * from shared memory and exits right after this test, before we set
-	 * the flag and send signal. And the PGPROC entry might even be recycled
-	 * by a new process, so it's remotely possible that we signal a wrong
-	 * process. That's OK, all the signals are such that no harm is done.
-	 */
-	pid = proc->pid;
-	if (pid == 0)
-		return;
-
-	/* Atomically set the proper flag */
-	proc->signalFlags[reason] = true;
-	/* Send SIGUSR1 to the process */
-	kill(pid, SIGUSR1);
-}
-
-/*
- * CheckProcSignal - check to see if the particular reason has been
- * signaled, and clear the signal flag.  Should be called after 
- * receiving SIGUSR1.
- */
-bool
-CheckProcSignal(ProcSignalReason reason)
-{
-	/* Careful here --- don't clear flag if we haven't seen it set */
-	if (MyProc->signalFlags[reason])
-	{
-		MyProc->signalFlags[reason] = false;
-		return true;
-	}
-
-	return false;
-}
-
 
 /*****************************************************************************
  * SIGALRM interrupt support

src/backend/tcop/postgres.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.559 2008/12/09 14:28:20 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.560 2008/12/09 15:59:39 heikki Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -2436,23 +2436,6 @@ drop_unnamed_stmt(void)
  * --------------------------------
  */
 
-/*
- * proc_sigusr1_handler - handle SIGUSR1 signal.
- *
- * SIGUSR1 is multiplexed to handle multiple different events. The signalFlags
- * array in PGPROC indicates which events have been signaled.
- */
-void
-proc_sigusr1_handler(SIGNAL_ARGS)
-{
-	int			save_errno = errno;
-
-	if (CheckProcSignal(PROCSIG_CATCHUP_INTERRUPT))
-		HandleCatchupInterrupt();
-
-	errno = save_errno;
-}
-
 /*
  * quickdie() occurs when signalled SIGQUIT by the postmaster.
  *
@@ -3197,7 +3180,7 @@ PostgresMain(int argc, char *argv[], const char *username)
 	 * of output during who-knows-what operation...
 	 */
 	pqsignal(SIGPIPE, SIG_IGN);
-	pqsignal(SIGUSR1, proc_sigusr1_handler);
+	pqsignal(SIGUSR1, CatchupInterruptHandler);
 	pqsignal(SIGUSR2, NotifyInterruptHandler);
 	pqsignal(SIGFPE, FloatExceptionHandler);
 

src/include/storage/proc.h

@@ -7,15 +7,13 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.108 2008/12/09 14:28:20 heikki Exp $
+ * $PostgreSQL: pgsql/src/include/storage/proc.h,v 1.109 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
 #ifndef _PROC_H_
 #define _PROC_H_
 
-#include <signal.h>
-
 #include "storage/lock.h"
 #include "storage/pg_sema.h"
 
@@ -40,19 +38,6 @@ struct XidCache
 	TransactionId xids[PGPROC_MAX_CACHED_SUBXIDS];
 };
 
-/*
- * Reasons for signaling a process (a backend or an auxiliary process, like
- * autovacuum worker). We can cope with simultaneous signals for different
- * reasons. If the same reason is signaled multiple times in quick succession,
- * however, the process is likely to observe only one notification of it.
- * This is okay for the present uses.
- */
-typedef enum
-{
-	PROCSIG_CATCHUP_INTERRUPT,	/* catchup interrupt */
-	NUM_PROCSIGNALS				/* Must be last value of enum! */
-} ProcSignalReason;
-
 /* Flags for PGPROC->vacuumFlags */
 #define		PROC_IS_AUTOVACUUM	0x01	/* is it an autovac worker? */
 #define		PROC_IN_VACUUM		0x02	/* currently running lazy vacuum */
@@ -108,16 +93,6 @@ struct PGPROC
 
 	uint8		vacuumFlags;	/* vacuum-related flags, see above */
 
-	/*
-	 * SIGUSR1 signal is multiplexed for multiple purposes. signalFlags
-	 * indicates which "reasons" have been signaled.
-	 *
-	 * The flags are declared as "volatile sig_atomic_t" for maximum
-	 * portability.  This should ensure that loads and stores of the flag
-	 * values are atomic, allowing us to dispense with any explicit locking.
-	 */
-	volatile sig_atomic_t signalFlags[NUM_PROCSIGNALS];
-
 	/* Info about LWLock the process is currently waiting for, if any. */
 	bool		lwWaiting;		/* true if waiting for an LW lock */
 	bool		lwExclusive;	/* true if waiting for exclusive access */
@@ -196,9 +171,6 @@ extern void LockWaitCancel(void);
 extern void ProcWaitForSignal(void);
 extern void ProcSendSignal(int pid);
 
-extern void SendProcSignal(PGPROC *proc, ProcSignalReason reason);
-extern bool CheckProcSignal(ProcSignalReason reason);
-
 extern bool enable_sig_alarm(int delayms, bool is_statement_timeout);
 extern bool disable_sig_alarm(bool is_statement_timeout);
 extern void handle_sig_alarm(SIGNAL_ARGS);

src/include/storage/sinval.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/sinval.h,v 1.49 2008/12/09 14:28:20 heikki Exp $
+ * $PostgreSQL: pgsql/src/include/storage/sinval.h,v 1.50 2008/12/09 15:59:39 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -90,7 +90,7 @@ extern void ReceiveSharedInvalidMessages(
 							 void (*resetFunction) (void));
 
 /* signal handler for catchup events (SIGUSR1) */
-extern void HandleCatchupInterrupt(void);
+extern void CatchupInterruptHandler(SIGNAL_ARGS);
 
 /*
  * enable/disable processing of catchup events directly from signal handler.

src/include/tcop/tcopprot.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/tcop/tcopprot.h,v 1.94 2008/12/09 14:28:20 heikki Exp $
+ * $PostgreSQL: pgsql/src/include/tcop/tcopprot.h,v 1.95 2008/12/09 15:59:39 heikki Exp $
  *
  * OLD COMMENTS
  *	  This file was created so that other c files could get the two
@@ -56,7 +56,6 @@ extern List *pg_plan_queries(List *querytrees, int cursorOptions,
 
 extern bool assign_max_stack_depth(int newval, bool doit, GucSource source);
 
-extern void proc_sigusr1_handler(SIGNAL_ARGS);
 extern void die(SIGNAL_ARGS);
 extern void quickdie(SIGNAL_ARGS);
 extern void authdie(SIGNAL_ARGS);