Hi, On Wed, Dec 10, 2008 at 1:43 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > Heikki Linnakangas <[EMAIL PROTECTED]> writes: >> I'm surprised you feel that way. You suggested earlier >> (http://archives.postgresql.org/message-id/[EMAIL PROTECTED]) >> that a solution that only works for processes attached to shared memory >> would probably suffice for now. > > Well, I wasn't complaining about the dependence on being attached to > shared memory. What I'm complaining about is the dependence on the > rather complex PGPROC data structure. > >> That seems hard, considering that we also want it to work without >> locking. Hmm, I presume we can use spinlocks in a signal handler? >> Perhaps some sort of a hash table protected by a spinlock would work. > > No, locks are right out if the postmaster is supposed to be able to use > it. What I was thinking of is a simple linear array of PIDs and > sig_atomic_t flags. The slots could be assigned on the basis of > backendid, but callers trying to send a signal would have to scan the > array looking for the matching PID. (This doesn't seem outlandishly > expensive considering that one is about to do a kernel call anyway. > You might be able to save a few cycles by having the PID array separate > from the flag array, which should improve the cache friendliness of the > scan.) Also, for those callers who do have access to a PGPROC, there > could be a separate entry point that passes backendid instead of PID to > eliminate the search.
Thanks for the comment! I updated the patch so. Is this patch ready to apply? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
diff -rc base/src/backend/bootstrap/bootstrap.c new/src/backend/bootstrap/bootstrap.c *** base/src/backend/bootstrap/bootstrap.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/bootstrap/bootstrap.c 2008-12-10 17:16:23.000000000 +0900 *************** *** 389,394 **** --- 389,403 ---- InitAuxiliaryProcess(); #endif + /* + * Assign backend ID to auxiliary processes like backends, in order to + * allow multiplexing signal to auxiliary processes. Since backends use + * ID in the range from 1 to MaxBackends, we assign auxiliary processes + * with MaxBackends + AuxProcType as an unique ID. + */ + MyBackendId = MaxBackends + auxType; + MyProc->backendId = MyBackendId; + /* finish setting up bufmgr.c */ InitBufferPoolBackend(); diff -rc base/src/backend/commands/async.c new/src/backend/commands/async.c *** base/src/backend/commands/async.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/commands/async.c 2008-12-10 17:27:31.000000000 +0900 *************** *** 915,923 **** * a frontend command. Signal handler execution of inbound notifies * is disabled until the next EnableNotifyInterrupt call. * ! * 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) --- 915,924 ---- * 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. */ bool DisableNotifyInterrupt(void) *************** *** 954,960 **** nulls[Natts_pg_listener]; bool catchup_enabled; ! /* Must prevent SIGUSR1 interrupt while I am running */ catchup_enabled = DisableCatchupInterrupt(); if (Trace_notify) --- 955,961 ---- nulls[Natts_pg_listener]; bool catchup_enabled; ! /* Must prevent catchup interrupt while I am running */ catchup_enabled = DisableCatchupInterrupt(); if (Trace_notify) diff -rc base/src/backend/postmaster/autovacuum.c new/src/backend/postmaster/autovacuum.c *** base/src/backend/postmaster/autovacuum.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/postmaster/autovacuum.c 2008-12-10 17:28:05.000000000 +0900 *************** *** 1477,1483 **** pqsignal(SIGALRM, handle_sig_alarm); pqsignal(SIGPIPE, SIG_IGN); ! pqsignal(SIGUSR1, CatchupInterruptHandler); /* We don't listen for async notifies */ pqsignal(SIGUSR2, SIG_IGN); pqsignal(SIGFPE, FloatExceptionHandler); --- 1477,1483 ---- pqsignal(SIGALRM, handle_sig_alarm); pqsignal(SIGPIPE, SIG_IGN); ! pqsignal(SIGUSR1, proc_sigusr1_handler); /* We don't listen for async notifies */ pqsignal(SIGUSR2, SIG_IGN); pqsignal(SIGFPE, FloatExceptionHandler); diff -rc base/src/backend/storage/ipc/Makefile new/src/backend/storage/ipc/Makefile *** base/src/backend/storage/ipc/Makefile 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/storage/ipc/Makefile 2008-12-10 17:24:58.000000000 +0900 *************** *** 15,21 **** endif endif ! OBJS = ipc.o ipci.o pmsignal.o procarray.o shmem.o shmqueue.o \ sinval.o sinvaladt.o include $(top_srcdir)/src/backend/common.mk --- 15,21 ---- endif endif ! OBJS = ipc.o ipci.o procsignal.o pmsignal.o procarray.o shmem.o shmqueue.o \ sinval.o sinvaladt.o include $(top_srcdir)/src/backend/common.mk diff -rc base/src/backend/storage/ipc/ipci.c new/src/backend/storage/ipc/ipci.c *** base/src/backend/storage/ipc/ipci.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/storage/ipc/ipci.c 2008-12-10 18:14:15.000000000 +0900 *************** *** 205,210 **** --- 205,211 ---- * Set up interprocess signaling mechanisms */ PMSignalInit(); + ProcSignalInit(); BgWriterShmemInit(); AutoVacuumShmemInit(); diff -rc base/src/backend/storage/ipc/procsignal.c new/src/backend/storage/ipc/procsignal.c *** base/src/backend/storage/ipc/procsignal.c 2008-12-10 19:26:37.000000000 +0900 --- new/src/backend/storage/ipc/procsignal.c 2008-12-10 19:12:34.000000000 +0900 *************** *** 0 **** --- 1,127 ---- + /*------------------------------------------------------------------------- + * + * procsignal.c + * routines for signaling the backend, autovacuum worker and auxiliary process + * + * + * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION + * $PostgreSQL: pgsql/src/backend/storage/ipc/procsignal.c,v 1.25 2008/01/01 19:45:51 momjian Exp $ + * + *------------------------------------------------------------------------- + */ + #include "postgres.h" + + #include <signal.h> + + #include "bootstrap/bootstrap.h" + #include "miscadmin.h" + #include "storage/procsignal.h" + + + /* + * The backend, autovacuum worker and auxiliary process are sent SIGUSR1 + * with the specific reason, which is communicated via flags in shared + * memory. We keep a boolean flag for each possible "reason", so that + * different reasons can be signaled by different processes at the same + * time. (However, if the same reason is signaled more than once + * simultaneously, the backend, autovacuum worker and auxiliary process + * will observe it only once.) + * + * The flags are actually 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. + */ + typedef struct ProcSignalData { + pid_t pid; + volatile sig_atomic_t flags[NUM_PROCSIGNALS]; + } ProcSignalData; + + static ProcSignalData *ProcSignal; + static int MaxProcSignal; + + /* + * ProcSignalInit - initialize during shared-memory creation + */ + void + ProcSignalInit(void) + { + bool found; + + MaxProcSignal = MaxBackends + NUM_AUXPROCTYPE; + + ProcSignal = (ProcSignalData *) + ShmemInitStruct("ProcSignal", + MaxProcSignal * sizeof(ProcSignal), + &found); + + if (!found) + MemSet(ProcSignal, 0, MaxProcSignal * sizeof(ProcSignal)); + } + + /* + * SendProcSignal - signal the process (such as the backend, autovacuum + * worker and auxiliary process) identified by backend ID or pid. + */ + void + SendProcSignal(BackendId bid, pid_t pid, ProcSignalReason reason) + { + ProcSignalData *procSig = NULL; + int index; + + Assert(bid < MaxProcSignal); + + /* + * Since ProcSignal array is assigned on the basis of backend ID, + * we can get the target slot immediately if backend ID is passed. + */ + if (bid != InvalidBackendId) + procSig = &ProcSignal[bid]; + else + { + if (pid == 0) /* never match dummy ProcSignal */ + return; + + for (index = 0; index < MaxProcSignal; index++) + { + ProcSignalData *psig = &ProcSignal[index]; + + if (psig->pid == pid) + { + procSig = psig; + break; + } + } + + /* don't signal if not found */ + if (procSig == NULL) + return; + } + + /* Atomically set the proper flag */ + procSig->flags[reason] = true; + + /* Send signal to the process */ + kill(procSig->pid, SIGUSR1); + } + + /* + * CheckProcSignal - check to see if a particular reason has been + * signaled, and clear the signal flag. Should be called after + * receiving SIGUSR1. + */ + bool + CheckProcSignal(ProcSignalReason reason) + { + ProcSignalData *procSig = &ProcSignal[MyBackendId]; + + /* Careful here --- don't clear flag if we haven't seen it set */ + if (procSig->flags[reason]) + { + procSig->flags[reason] = false; + return true; + } + return false; + } diff -rc base/src/backend/storage/ipc/sinval.c new/src/backend/storage/ipc/sinval.c *** base/src/backend/storage/ipc/sinval.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/storage/ipc/sinval.c 2008-12-10 17:32:06.000000000 +0900 *************** *** 27,33 **** * 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 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 --- 27,34 ---- * 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 SIGUSR1 ! * with PROCSIG_CATCHUP_INTERRUPT 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,152 **** /* ! * CatchupInterruptHandler * ! * 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 --- 145,154 ---- /* ! * HandleCatchupInterrupt * ! * This is called when SIGUSR1 with PROCSIG_CATCHUP_INTERRUPT is ! * received. * * If we are idle (catchupInterruptEnabled is set), we can safely * invoke ProcessCatchupEvent directly. Otherwise, just set a flag *************** *** 156,168 **** * since there's no longer any reason to do anything.) */ void ! CatchupInterruptHandler(SIGNAL_ARGS) { - int save_errno = errno; - /* ! * Note: this is a SIGNAL HANDLER. You must be very wary what you do ! * here. */ /* Don't joggle the elbow of proc_exit */ --- 158,168 ---- * since there's no longer any reason to do anything.) */ void ! HandleCatchupInterrupt(void) { /* ! * Note: this is called by a SIGNAL HANDLER. ! * You must be very wary what you do here. */ /* Don't joggle the elbow of proc_exit */ *************** *** 216,223 **** */ catchupInterruptOccurred = 1; } - - errno = save_errno; } /* --- 216,221 ---- *************** *** 289,295 **** /* * ProcessCatchupEvent * ! * 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 --- 287,294 ---- /* * ProcessCatchupEvent * ! * Respond to a catchup event (SIGUSR1 with PROCSIG_CATCHUP_INTERRUPT) ! * 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 -rc base/src/backend/storage/ipc/sinvaladt.c new/src/backend/storage/ipc/sinvaladt.c *** base/src/backend/storage/ipc/sinvaladt.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/storage/ipc/sinvaladt.c 2008-12-10 19:13:06.000000000 +0900 *************** *** 21,26 **** --- 21,27 ---- #include "storage/backendid.h" #include "storage/ipc.h" #include "storage/proc.h" + #include "storage/procsignal.h" #include "storage/shmem.h" #include "storage/sinvaladt.h" #include "storage/spin.h" *************** *** 644,650 **** segP->nextThreshold = (numMsgs / CLEANUP_QUANTUM + 1) * CLEANUP_QUANTUM; /* ! * 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) --- 645,651 ---- 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. */ if (needSig) *************** *** 655,661 **** LWLockRelease(SInvalReadLock); LWLockRelease(SInvalWriteLock); elog(DEBUG4, "sending sinval catchup signal to PID %d", (int) his_pid); ! kill(his_pid, SIGUSR1); if (callerHasWriteLock) LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); } --- 656,662 ---- LWLockRelease(SInvalReadLock); LWLockRelease(SInvalWriteLock); elog(DEBUG4, "sending sinval catchup signal to PID %d", (int) his_pid); ! SendProcSignal(InvalidBackendId, his_pid, SIGUSR1); if (callerHasWriteLock) LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); } diff -rc base/src/backend/tcop/postgres.c new/src/backend/tcop/postgres.c *** base/src/backend/tcop/postgres.c 2008-12-10 16:29:10.000000000 +0900 --- new/src/backend/tcop/postgres.c 2008-12-10 19:13:42.000000000 +0900 *************** *** 59,64 **** --- 59,65 ---- #include "storage/bufmgr.h" #include "storage/ipc.h" #include "storage/proc.h" + #include "storage/procsignal.h" #include "storage/sinval.h" #include "tcop/fastpath.h" #include "tcop/pquery.h" *************** *** 2437,2442 **** --- 2438,2465 ---- */ /* + * 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)) + { + /* + * Catchup interrupt has been sent. + */ + HandleCatchupInterrupt(); + } + + errno = save_errno; + } + + /* * quickdie() occurs when signalled SIGQUIT by the postmaster. * * Some backend has bought the farm, *************** *** 3180,3186 **** * of output during who-knows-what operation... */ pqsignal(SIGPIPE, SIG_IGN); ! pqsignal(SIGUSR1, CatchupInterruptHandler); pqsignal(SIGUSR2, NotifyInterruptHandler); pqsignal(SIGFPE, FloatExceptionHandler); --- 3203,3209 ---- * of output during who-knows-what operation... */ pqsignal(SIGPIPE, SIG_IGN); ! pqsignal(SIGUSR1, proc_sigusr1_handler); pqsignal(SIGUSR2, NotifyInterruptHandler); pqsignal(SIGFPE, FloatExceptionHandler); diff -rc base/src/include/bootstrap/bootstrap.h new/src/include/bootstrap/bootstrap.h *** base/src/include/bootstrap/bootstrap.h 2008-12-10 16:29:10.000000000 +0900 --- new/src/include/bootstrap/bootstrap.h 2008-12-10 18:08:12.000000000 +0900 *************** *** 70,76 **** BootstrapProcess, StartupProcess, BgWriterProcess, ! WalWriterProcess } AuxProcType; #endif /* BOOTSTRAP_H */ --- 70,78 ---- BootstrapProcess, StartupProcess, BgWriterProcess, ! WalWriterProcess, ! ! NUM_AUXPROCTYPE } AuxProcType; #endif /* BOOTSTRAP_H */ diff -rc base/src/include/storage/procsignal.h new/src/include/storage/procsignal.h *** base/src/include/storage/procsignal.h 2008-12-10 19:26:52.000000000 +0900 --- new/src/include/storage/procsignal.h 2008-12-10 19:07:11.000000000 +0900 *************** *** 0 **** --- 1,41 ---- + /*------------------------------------------------------------------------- + * + * procsignal.h + * routines for signaling the backend, autovacuum worker and auxiliary process + * + * + * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * $PostgreSQL: pgsql/src/include/storage/procsignal.h,v 1.20 2008/06/19 21:32:56 tgl Exp $ + * + *------------------------------------------------------------------------- + */ + #ifndef PROCSIGNAL_H + #define PROCSIGNAL_H + + #include "storage/backendid.h" + + /* + * Reasons for signaling the process (such as backend, autovacuum worker + * and auxiliary process). We can cope with simultaneous signals for + * different reasons. If the same reason is signaled multiple times in + * quick succession, however, the postmaster 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; + + + /* + * prototypes for functions in procsignal.c + */ + extern void ProcSignalInit(void); + extern void SendProcSignal(BackendId bid, pid_t pid, ProcSignalReason reason); + extern bool CheckProcSignal(ProcSignalReason reason); + + #endif /* PMSIGNAL_H */ diff -rc base/src/include/storage/sinval.h new/src/include/storage/sinval.h *** base/src/include/storage/sinval.h 2008-12-10 16:29:10.000000000 +0900 --- new/src/include/storage/sinval.h 2008-12-10 18:53:51.000000000 +0900 *************** *** 90,96 **** void (*resetFunction) (void)); /* signal handler for catchup events (SIGUSR1) */ ! extern void CatchupInterruptHandler(SIGNAL_ARGS); /* * enable/disable processing of catchup events directly from signal handler. --- 90,96 ---- void (*resetFunction) (void)); /* signal handler for catchup events (SIGUSR1) */ ! extern void HandleCatchupInterrupt(void); /* * enable/disable processing of catchup events directly from signal handler. diff -rc base/src/include/tcop/tcopprot.h new/src/include/tcop/tcopprot.h *** base/src/include/tcop/tcopprot.h 2008-12-10 16:29:10.000000000 +0900 --- new/src/include/tcop/tcopprot.h 2008-12-10 18:54:26.000000000 +0900 *************** *** 56,61 **** --- 56,62 ---- 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);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers