From dea81a6cf620528e6c17e9973b32b54e0076cf06 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Tue, 9 Dec 2008 15:59:39 +0000 Subject: [PATCH] Revert SIGUSR1 multiplexing patch, per Tom's objection. --- src/backend/access/transam/twophase.c | 3 +- src/backend/commands/async.c | 11 +++--- src/backend/postmaster/autovacuum.c | 4 +-- src/backend/storage/ipc/sinval.c | 23 ++++++------ src/backend/storage/ipc/sinvaladt.c | 37 +++++++++---------- src/backend/storage/lmgr/proc.c | 52 +-------------------------- src/backend/tcop/postgres.c | 21 ++--------- src/include/storage/proc.h | 30 +--------------- src/include/storage/sinval.h | 4 +-- src/include/tcop/tcopprot.h | 3 +- 10 files changed, 45 insertions(+), 143 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index c500144ca9..39906d3c1f 100644 --- a/src/backend/access/transam/twophase.c +++ b/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; diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 0767d97ef9..f1e0b4bcf5 100644 --- a/src/backend/commands/async.c +++ b/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 - * PROCSIG_CATCHUP_INTERRUPT is received, so as to prevent conflicts - * if one signal interrupts the other. So we must return the previous - * state of the flag. + * The SIGUSR1 signal handler also needs to call this, so as to + * prevent conflicts if one signal interrupts the other. So we + * must return the previous 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) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 31b5eeca5f..1d9e2068ca 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/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); diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index d002f45489..c8d3a114c4 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/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 - * PROCSIG_CATCHUP_INTERRUPT to any backend that gets too far behind. + * through a cache reset exercise. This is done by sending SIGUSR1 + * 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. - * You must be very wary what you do here. + * Note: this is a SIGNAL HANDLER. You must be very wary what you do + * 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 - * backend. + * Respond to a catchup event (SIGUSR1) from another backend. * * This is called either directly from the SIGUSR1 signal handler, * or the next time control reaches the outer idle loop (assuming diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index fe9ff43ae7..82dd20dedf 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/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. */ - PGPROC *proc; /* PGPROC entry of backend, for signaling */ - /* nextMsgNum is meaningless if proc == NULL or resetState is true. */ + /* procPid is zero in an inactive ProcState array entry. */ + pid_t procPid; /* PID of backend, for signaling */ + /* 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 - * SendProcSignal() might not be fast, we don't want to hold locks while - * executing it. + * Lastly, signal anyone who needs a catchup interrupt. Since kill() + * might not be fast, we don't want to hold locks while 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", - (int) his_proc->pid); - SendProcSignal(his_proc, PROCSIG_CATCHUP_INTERRUPT); + elog(DEBUG4, "sending sinval catchup signal to PID %d", (int) his_pid); + kill(his_pid, SIGUSR1); if (callerHasWriteLock) LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); } diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 77388f86ae..4ee651e306 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/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 diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 6df02a3f79..b09401652b 100644 --- a/src/backend/tcop/postgres.c +++ b/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); diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 3ff484ee41..4494fbaad8 100644 --- a/src/include/storage/proc.h +++ b/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 - #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); diff --git a/src/include/storage/sinval.h b/src/include/storage/sinval.h index af47f8de93..5722fac353 100644 --- a/src/include/storage/sinval.h +++ b/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. diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 3a2f4fc2e6..bd458fac9a 100644 --- a/src/include/tcop/tcopprot.h +++ b/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); -- 2.40.0