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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
