>From 9b9ffc8377bc90075932321eed23636d36a46ce8 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sat, 27 Sep 2014 23:49:30 +0200 Subject: [PATCH 3/4] Move sinval catchup and notify processing out of signal handlers. Author: Andres Freund --- src/backend/commands/async.c | 186 ++++++---------------------------- src/backend/libpq/be-secure-openssl.c | 25 +++-- src/backend/libpq/be-secure.c | 28 +++-- src/backend/postmaster/autovacuum.c | 6 +- src/backend/storage/ipc/sinval.c | 166 ++++-------------------------- src/backend/tcop/postgres.c | 103 ++++++------------- src/include/commands/async.h | 12 +-- src/include/storage/sinval.h | 7 +- src/include/tcop/tcopprot.h | 4 +- 9 files changed, 137 insertions(+), 400 deletions(-) diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 92f2077..05e02c3 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -126,6 +126,7 @@ #include "miscadmin.h" #include "storage/ipc.h" #include "storage/lmgr.h" +#include "storage/proc.h" #include "storage/procarray.h" #include "storage/procsignal.h" #include "storage/sinval.h" @@ -334,17 +335,13 @@ static List *pendingNotifies = NIL; /* list of Notifications */ static List *upperPendingNotifies = NIL; /* list of upper-xact lists */ /* - * State for inbound notifications consists of two flags: one saying whether - * the signal handler is currently allowed to call ProcessIncomingNotify - * directly, and one saying whether the signal has occurred but the handler - * was not allowed to call ProcessIncomingNotify at the time. - * - * NB: the "volatile" on these declarations is critical! If your compiler - * does not grok "volatile", you'd be best advised to compile this file - * with all optimization turned off. + * Inbound notifications are initially processed by HandleNotifyInterrupt(), + * called from inside a signal handler. That just sets the + * notifyInterruptPending flag and sets the process + * latch. ProcessNotifyInterrupt() will then be called whenever it's safe to + * actually deal with the interrupt. */ -static volatile sig_atomic_t notifyInterruptEnabled = 0; -static volatile sig_atomic_t notifyInterruptOccurred = 0; +volatile sig_atomic_t notifyInterruptPending = 0; /* True if we've registered an on_shmem_exit cleanup */ static bool unlistenExitRegistered = false; @@ -1625,11 +1622,10 @@ AtSubAbort_Notify(void) /* * HandleNotifyInterrupt * - * This is called when PROCSIG_NOTIFY_INTERRUPT is received. - * - * If we are idle (notifyInterruptEnabled is set), we can safely invoke - * ProcessIncomingNotify directly. Otherwise, just set a flag - * to do it later. + * Signal handler portion of interrupt handling. Let the backend know + * that there's a pending notify interrupt. If we're currently reading + * from the client, this will interrupt the read and + * ProcessClientReadInterrupt() will call ProcessNotifyInterrupt(). */ void HandleNotifyInterrupt(void) @@ -1641,148 +1637,35 @@ HandleNotifyInterrupt(void) * they were ever turned on. */ - /* Don't joggle the elbow of proc_exit */ - if (proc_exit_inprogress) - return; - - if (notifyInterruptEnabled) - { - bool save_ImmediateInterruptOK = ImmediateInterruptOK; - - /* - * We may be called while ImmediateInterruptOK is true; turn it off - * while messing with the NOTIFY state. This prevents problems if - * SIGINT or similar arrives while we're working. Just to be real - * sure, bump the interrupt holdoff counter as well. That way, even - * if something inside ProcessIncomingNotify() transiently sets - * ImmediateInterruptOK (eg while waiting on a lock), we won't get - * interrupted until we're done with the notify interrupt. - */ - ImmediateInterruptOK = false; - HOLD_INTERRUPTS(); - - /* - * I'm not sure whether some flavors of Unix might allow another - * SIGUSR1 occurrence to recursively interrupt this routine. To cope - * with the possibility, we do the same sort of dance that - * EnableNotifyInterrupt must do --- see that routine for comments. - */ - notifyInterruptEnabled = 0; /* disable any recursive signal */ - notifyInterruptOccurred = 1; /* do at least one iteration */ - for (;;) - { - notifyInterruptEnabled = 1; - if (!notifyInterruptOccurred) - break; - notifyInterruptEnabled = 0; - if (notifyInterruptOccurred) - { - /* Here, it is finally safe to do stuff. */ - if (Trace_notify) - elog(DEBUG1, "HandleNotifyInterrupt: perform async notify"); - - ProcessIncomingNotify(); - - if (Trace_notify) - elog(DEBUG1, "HandleNotifyInterrupt: done"); - } - } + /* signal that work needs to be done */ + notifyInterruptPending = 1; - /* - * Restore the holdoff level and ImmediateInterruptOK, and check for - * interrupts if needed. - */ - RESUME_INTERRUPTS(); - ImmediateInterruptOK = save_ImmediateInterruptOK; - if (save_ImmediateInterruptOK) - CHECK_FOR_INTERRUPTS(); - } - else - { - /* - * In this path it is NOT SAFE to do much of anything, except this: - */ - notifyInterruptOccurred = 1; - } + /* make sure the event is processed in due course */ + if (MyProc != NULL) + SetLatch(&MyProc->procLatch); } - /* - * EnableNotifyInterrupt - * - * This is called by the PostgresMain main loop just before waiting - * for a frontend command. If we are truly idle (ie, *not* inside - * a transaction block), then process any pending inbound notifies, - * and enable the signal handler to process future notifies directly. + * ProcessNotifyInterrupt * - * NOTE: the signal handler starts out disabled, and stays so until - * PostgresMain calls this the first time. + * This is called just before/after waiting for a frontend command. If a + * interrupt arrives (via HandleNotifyInterrupt()) while reading, the + * read will be interrupted, and this will get called. If we are truly + * idle (ie, *not* inside a transaction block), process the incoming + * notifies. */ + void -EnableNotifyInterrupt(void) +ProcessNotifyInterrupt(void) { if (IsTransactionOrTransactionBlock()) return; /* not really idle */ - /* - * This code is tricky because we are communicating with a signal handler - * that could interrupt us at any point. If we just checked - * notifyInterruptOccurred and then set notifyInterruptEnabled, we could - * fail to respond promptly to a signal that happens in between those two - * steps. (A very small time window, perhaps, but Murphy's Law says you - * can hit it...) Instead, we first set the enable flag, then test the - * occurred flag. If we see an unserviced interrupt has occurred, we - * re-clear the enable flag before going off to do the service work. (That - * prevents re-entrant invocation of ProcessIncomingNotify() if another - * interrupt occurs.) If an interrupt comes in between the setting and - * clearing of notifyInterruptEnabled, then it will have done the service - * work and left notifyInterruptOccurred zero, so we have to check again - * after clearing enable. The whole thing has to be in a loop in case - * another interrupt occurs while we're servicing the first. Once we get - * out of the loop, enable is set and we know there is no unserviced - * interrupt. - * - * NB: an overenthusiastic optimizing compiler could easily break this - * code. Hopefully, they all understand what "volatile" means these days. - */ - for (;;) + while (notifyInterruptPending) { - notifyInterruptEnabled = 1; - if (!notifyInterruptOccurred) - break; - notifyInterruptEnabled = 0; - if (notifyInterruptOccurred) - { - if (Trace_notify) - elog(DEBUG1, "EnableNotifyInterrupt: perform async notify"); - - ProcessIncomingNotify(); - - if (Trace_notify) - elog(DEBUG1, "EnableNotifyInterrupt: done"); - } + ProcessIncomingNotify(); } } -/* - * DisableNotifyInterrupt - * - * This is called by the PostgresMain main loop just after receiving - * a frontend command. Signal handler execution of inbound notifies - * is disabled until the next EnableNotifyInterrupt call. - * - * The PROCSIG_CATCHUP_INTERRUPT 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) -{ - bool result = (notifyInterruptEnabled != 0); - - notifyInterruptEnabled = 0; - - return result; -} /* * Read all pending notifications from the queue, and deliver appropriate @@ -2076,9 +1959,10 @@ asyncQueueAdvanceTail(void) /* * ProcessIncomingNotify * - * Deal with arriving NOTIFYs from other backends. - * This is called either directly from the PROCSIG_NOTIFY_INTERRUPT - * signal handler, or the next time control reaches the outer idle loop. + * Deal with arriving NOTIFYs from other backends as soon as it's safe to + * do so. This used to be called from the PROCSIG_NOTIFY_INTERRUPT + * signal handler, but isn't anymore. + * * Scan the queue for arriving notifications and report them to my front * end. * @@ -2087,18 +1971,13 @@ asyncQueueAdvanceTail(void) static void ProcessIncomingNotify(void) { - bool catchup_enabled; - /* We *must* reset the flag */ - notifyInterruptOccurred = 0; + notifyInterruptPending = 0; /* Do nothing else if we aren't actively listening */ if (listenChannels == NIL) return; - /* Must prevent catchup interrupt while I am running */ - catchup_enabled = DisableCatchupInterrupt(); - if (Trace_notify) elog(DEBUG1, "ProcessIncomingNotify"); @@ -2123,9 +2002,6 @@ ProcessIncomingNotify(void) if (Trace_notify) elog(DEBUG1, "ProcessIncomingNotify: done"); - - if (catchup_enabled) - EnableCatchupInterrupt(); } /* diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 31fd004..6fc6903 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -374,6 +374,7 @@ aloop: { case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: + /* FIXME: interrupt handling? */ if (MyProc != NULL) WaitLatchOrSocket(&MyProc->procLatch, err == SSL_ERROR_WANT_READ ? @@ -518,6 +519,17 @@ rloop: break; case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: + /* + * We'll, among other situations, get here if the low level + * routine doing the actual recv() via the socket got interrupted + * by a signal. That's so we can handle interrupts once outside + * openssl so we don't jump out from underneath its covers. We can + * check this both, when reading and writing, because even when + * writing that's just openssl's doing, not a 'proper' write + * initiated by postgres. + */ + ProcessClientReadInterrupt(); + if (port->noblock) { errno = EWOULDBLOCK; @@ -626,12 +638,13 @@ wloop: break; case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: -#ifdef WIN32 - pgwin32_waitforsinglesocket(SSL_get_fd(port->ssl), - (err == SSL_ERROR_WANT_READ) ? - FD_READ | FD_CLOSE : FD_WRITE | FD_CLOSE, - INFINITE); -#endif + /* XXX: We likely will want to process some interrupts here */ + if (port->noblock) + { + errno = EWOULDBLOCK; + n = -1; + break; + } goto wloop; case SSL_ERROR_SYSCALL: /* leave it to caller to ereport the value of errno */ diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 605c2be..7b5b30f 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -129,6 +129,7 @@ secure_read(Port *port, void *ptr, size_t len) { ssize_t n; +retry: #ifdef USE_SSL if (port->ssl_in_use) { @@ -140,6 +141,14 @@ secure_read(Port *port, void *ptr, size_t len) n = secure_raw_read(port, ptr, len); } + /* Process interrupts that happened while (or before) receiving. */ + ProcessClientReadInterrupt(); + + /* retry after processing interrupts */ + if (n < 0 && errno == EINTR) + { + goto retry; + } return n; } @@ -148,8 +157,6 @@ secure_raw_read(Port *port, void *ptr, size_t len) { ssize_t n; - prepare_for_client_read(); - /* * Try to read from the socket without blocking. If it suceeds we're * done, otherwise we'll wait for the socket using the latch mechanism. @@ -164,15 +171,22 @@ rloop: Assert(MyProc); w = WaitLatchOrSocket(&MyProc->procLatch, - WL_SOCKET_READABLE, + WL_LATCH_SET | WL_SOCKET_READABLE, port->sock, 0); - if (w & WL_SOCKET_READABLE) + if (w & WL_LATCH_SET) + { + ResetLatch(&MyProc->procLatch); + /* + * Force a return, so interrupts can be processed when not + * (possibly) underneath a ssl library. + */ + errno = EINTR; + } + else if (w & WL_SOCKET_READABLE) goto rloop; } - client_read_ended(); - return n; } @@ -196,6 +210,8 @@ secure_write(Port *port, void *ptr, size_t len) n = secure_raw_write(port, ptr, len); } + /* XXX: We likely will want to process some interrupts here */ + return n; } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index c240d24..4a43c81 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -601,9 +601,6 @@ AutoVacLauncherMain(int argc, char *argv[]) launcher_determine_sleep(!dlist_is_empty(&AutoVacuumShmem->av_freeWorkers), false, &nap); - /* Allow sinval catchup interrupts while sleeping */ - EnableCatchupInterrupt(); - /* * Wait until naptime expires or we get some type of signal (all the * signal handlers will wake us by calling SetLatch). @@ -614,7 +611,8 @@ AutoVacLauncherMain(int argc, char *argv[]) ResetLatch(&MyProc->procLatch); - DisableCatchupInterrupt(); + /* Process sinval catchup interrupts that happened while sleeping */ + ProcessCatchupInterrupt(); /* * Emergency bailout if postmaster has died. This is to avoid the diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index d7d0406..307be49 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/src/backend/storage/ipc/sinval.c @@ -18,6 +18,7 @@ #include "commands/async.h" #include "miscadmin.h" #include "storage/ipc.h" +#include "storage/proc.h" #include "storage/sinvaladt.h" #include "utils/inval.h" @@ -32,17 +33,12 @@ uint64 SharedInvalidMessageCounter; * through a cache reset exercise. This is done by sending * 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 - * directly, and one saying whether the signal has occurred but the handler - * was not allowed to call ProcessCatchupEvent at the time. - * - * NB: the "volatile" on these declarations is critical! If your compiler - * does not grok "volatile", you'd be best advised to compile this file - * with all optimization turned off. + * The signal handler will set a interrupt pending flag and will set the + * processes latch. Whenever starting to read from the client, or when + * interrupted while doing so, ProcessClientReadInterrupt() will call + * ProcessCatchupEvent(). */ -static volatile int catchupInterruptEnabled = 0; -static volatile int catchupInterruptOccurred = 0; +volatile sig_atomic_t catchupInterruptPending = 0; static void ProcessCatchupEvent(void); @@ -141,9 +137,9 @@ ReceiveSharedInvalidMessages( * catchup signal this way avoids creating spikes in system load for what * should be just a background maintenance activity. */ - if (catchupInterruptOccurred) + if (catchupInterruptPending) { - catchupInterruptOccurred = 0; + catchupInterruptPending = 0; elog(DEBUG4, "sinval catchup complete, cleaning queue"); SICleanupQueue(false, 0); } @@ -155,12 +151,9 @@ ReceiveSharedInvalidMessages( * * This is called when PROCSIG_CATCHUP_INTERRUPT is received. * - * If we are idle (catchupInterruptEnabled is set), we can safely - * invoke ProcessCatchupEvent directly. Otherwise, just set a flag - * to do it later. (Note that it's quite possible for normal processing - * of the current transaction to cause ReceiveSharedInvalidMessages() - * to be run later on; in that case the flag will get cleared again, - * since there's no longer any reason to do anything.) + * We used to directly call ProcessCatchupEvent directly when idle. These days + * we just set a flag to do it later and notify the process of that fact by + * setting the processes latch. */ void HandleCatchupInterrupt(void) @@ -170,153 +163,37 @@ HandleCatchupInterrupt(void) * you do here. */ - /* Don't joggle the elbow of proc_exit */ - if (proc_exit_inprogress) - return; - - if (catchupInterruptEnabled) - { - bool save_ImmediateInterruptOK = ImmediateInterruptOK; - - /* - * We may be called while ImmediateInterruptOK is true; turn it off - * while messing with the catchup state. This prevents problems if - * SIGINT or similar arrives while we're working. Just to be real - * sure, bump the interrupt holdoff counter as well. That way, even - * if something inside ProcessCatchupEvent() transiently sets - * ImmediateInterruptOK (eg while waiting on a lock), we won't get - * interrupted until we're done with the catchup interrupt. - */ - ImmediateInterruptOK = false; - HOLD_INTERRUPTS(); - - /* - * I'm not sure whether some flavors of Unix might allow another - * SIGUSR1 occurrence to recursively interrupt this routine. To cope - * with the possibility, we do the same sort of dance that - * EnableCatchupInterrupt must do --- see that routine for comments. - */ - catchupInterruptEnabled = 0; /* disable any recursive signal */ - catchupInterruptOccurred = 1; /* do at least one iteration */ - for (;;) - { - catchupInterruptEnabled = 1; - if (!catchupInterruptOccurred) - break; - catchupInterruptEnabled = 0; - if (catchupInterruptOccurred) - { - /* Here, it is finally safe to do stuff. */ - ProcessCatchupEvent(); - } - } + catchupInterruptPending = 1; - /* - * Restore the holdoff level and ImmediateInterruptOK, and check for - * interrupts if needed. - */ - RESUME_INTERRUPTS(); - ImmediateInterruptOK = save_ImmediateInterruptOK; - if (save_ImmediateInterruptOK) - CHECK_FOR_INTERRUPTS(); - } - else - { - /* - * In this path it is NOT SAFE to do much of anything, except this: - */ - catchupInterruptOccurred = 1; - } + /* make sure the event is processed in due course */ + if (MyProc != NULL) + SetLatch(&MyProc->procLatch); } -/* - * EnableCatchupInterrupt - * - * This is called by the PostgresMain main loop just before waiting - * for a frontend command. We process any pending catchup events, - * and enable the signal handler to process future events directly. - * - * NOTE: the signal handler starts out disabled, and stays so until - * PostgresMain calls this the first time. - */ void -EnableCatchupInterrupt(void) +ProcessCatchupInterrupt(void) { - /* - * This code is tricky because we are communicating with a signal handler - * that could interrupt us at any point. If we just checked - * catchupInterruptOccurred and then set catchupInterruptEnabled, we could - * fail to respond promptly to a signal that happens in between those two - * steps. (A very small time window, perhaps, but Murphy's Law says you - * can hit it...) Instead, we first set the enable flag, then test the - * occurred flag. If we see an unserviced interrupt has occurred, we - * re-clear the enable flag before going off to do the service work. (That - * prevents re-entrant invocation of ProcessCatchupEvent() if another - * interrupt occurs.) If an interrupt comes in between the setting and - * clearing of catchupInterruptEnabled, then it will have done the service - * work and left catchupInterruptOccurred zero, so we have to check again - * after clearing enable. The whole thing has to be in a loop in case - * another interrupt occurs while we're servicing the first. Once we get - * out of the loop, enable is set and we know there is no unserviced - * interrupt. - * - * NB: an overenthusiastic optimizing compiler could easily break this - * code. Hopefully, they all understand what "volatile" means these days. - */ for (;;) { - catchupInterruptEnabled = 1; - if (!catchupInterruptOccurred) + if (!catchupInterruptPending) break; - catchupInterruptEnabled = 0; - if (catchupInterruptOccurred) - ProcessCatchupEvent(); + ProcessCatchupEvent(); } } /* - * DisableCatchupInterrupt - * - * This is called by the PostgresMain main loop just after receiving - * a frontend command. Signal handler execution of catchup events - * is disabled until the next EnableCatchupInterrupt call. - * - * The PROCSIG_NOTIFY_INTERRUPT 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 -DisableCatchupInterrupt(void) -{ - bool result = (catchupInterruptEnabled != 0); - - catchupInterruptEnabled = 0; - - return result; -} - -/* * ProcessCatchupEvent * * Respond to a catchup event (PROCSIG_CATCHUP_INTERRUPT) from another - * backend. - * - * This is called either directly from the PROCSIG_CATCHUP_INTERRUPT - * signal handler, or the next time control reaches the outer idle loop - * (assuming there's still anything to do by then). + * backend once it's safe to do so. */ static void ProcessCatchupEvent(void) { - bool notify_enabled; - - /* Must prevent notify interrupt while I am running */ - notify_enabled = DisableNotifyInterrupt(); - /* * What we need to do here is cause ReceiveSharedInvalidMessages() to run, * which will do the necessary work and also reset the - * catchupInterruptOccurred flag. If we are inside a transaction we can + * catchupInterruptPending flag. If we are inside a transaction we can * just call AcceptInvalidationMessages() to do this. If we aren't, we * start and immediately end a transaction; the call to * AcceptInvalidationMessages() happens down inside transaction start. @@ -337,7 +214,4 @@ ProcessCatchupEvent(void) StartTransactionCommand(); CommitTransactionCommand(); } - - if (notify_enabled) - EnableNotifyInterrupt(); } diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index b3a332e..3a6aa1c 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -302,17 +302,23 @@ InteractiveBackend(StringInfo inBuf) * interactive_getc -- collect one character from stdin * * Even though we are not reading from a "client" process, we still want to - * respond to signals, particularly SIGTERM/SIGQUIT. Hence we must use - * prepare_for_client_read and client_read_ended. + * respond to signals, particularly SIGTERM/SIGQUIT. FIXME. */ static int interactive_getc(void) { int c; - prepare_for_client_read(); + /* + * FIXME: this will not process catchup interrupts or notifications. But + * those can't really be relevant for a standalone backend? + */ + ProcessClientReadInterrupt(); + c = getc(stdin); - client_read_ended(); + + ProcessClientReadInterrupt(); + return c; } @@ -487,50 +493,30 @@ ReadCommand(StringInfo inBuf) } /* - * prepare_for_client_read -- set up to possibly block on client input + * ProcessClientReadInterrupt() - Process interrupts specific to client reads * - * This must be called immediately before any low-level read from the - * client connection. It is necessary to do it at a sufficiently low level - * that there won't be any other operations except the read kernel call - * itself between this call and the subsequent client_read_ended() call. - * In particular there mustn't be use of malloc() or other potentially - * non-reentrant libc functions. This restriction makes it safe for us - * to allow interrupt service routines to execute nontrivial code while - * we are waiting for input. - */ -void -prepare_for_client_read(void) -{ - if (DoingCommandRead) - { - /* Enable immediate processing of asynchronous signals */ - EnableNotifyInterrupt(); - EnableCatchupInterrupt(); - - /* Allow cancel/die interrupts to be processed while waiting */ - ImmediateInterruptOK = true; - - /* And don't forget to detect one that already arrived */ - CHECK_FOR_INTERRUPTS(); - } -} - -/* - * client_read_ended -- get out of the client-input state + * This is called just after low-level reads. That might be after the read + * finished successfully, or it was interrupted via interrupt. * - * This is called just after low-level reads. It must preserve errno! + * Must preserve errno! */ void -client_read_ended(void) +ProcessClientReadInterrupt(void) { if (DoingCommandRead) { int save_errno = errno; - ImmediateInterruptOK = false; + /* Check for general interrupts that arrived while reading */ + CHECK_FOR_INTERRUPTS(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); + /* Process sinval catchup interrupts that happened while reading */ + if (catchupInterruptPending) + ProcessCatchupInterrupt(); + + /* Process sinval catchup interrupts that happened while reading */ + if (notifyInterruptPending) + ProcessNotifyInterrupt(); errno = save_errno; } @@ -2588,8 +2574,8 @@ die(SIGNAL_ARGS) ProcDiePending = true; /* - * If it's safe to interrupt, and we're waiting for input or a lock, - * service the interrupt immediately + * If it's safe to interrupt, and we're waiting for a lock, service + * the interrupt immediately */ if (ImmediateInterruptOK && InterruptHoldoffCount == 0 && CritSectionCount == 0) @@ -2598,8 +2584,6 @@ die(SIGNAL_ARGS) /* until we are done getting ready for it */ InterruptHoldoffCount++; LockErrorCleanup(); /* prevent CheckDeadLock from running */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); InterruptHoldoffCount--; ProcessInterrupts(); } @@ -2630,8 +2614,8 @@ StatementCancelHandler(SIGNAL_ARGS) QueryCancelPending = true; /* - * If it's safe to interrupt, and we're waiting for input or a lock, - * service the interrupt immediately + * If it's safe to interrupt, and we're waiting for a lock, service + * the interrupt immediately */ if (ImmediateInterruptOK && InterruptHoldoffCount == 0 && CritSectionCount == 0) @@ -2640,8 +2624,6 @@ StatementCancelHandler(SIGNAL_ARGS) /* until we are done getting ready for it */ InterruptHoldoffCount++; LockErrorCleanup(); /* prevent CheckDeadLock from running */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); InterruptHoldoffCount--; ProcessInterrupts(); } @@ -2789,8 +2771,8 @@ RecoveryConflictInterrupt(ProcSignalReason reason) RecoveryConflictRetryable = false; /* - * If it's safe to interrupt, and we're waiting for input or a lock, - * service the interrupt immediately + * If it's safe to interrupt, and we're waiting for a lock, service + * the interrupt immediately */ if (ImmediateInterruptOK && InterruptHoldoffCount == 0 && CritSectionCount == 0) @@ -2799,8 +2781,6 @@ RecoveryConflictInterrupt(ProcSignalReason reason) /* until we are done getting ready for it */ InterruptHoldoffCount++; LockErrorCleanup(); /* prevent CheckDeadLock from running */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); InterruptHoldoffCount--; ProcessInterrupts(); } @@ -2838,8 +2818,6 @@ ProcessInterrupts(void) ProcDiePending = false; QueryCancelPending = false; /* ProcDie trumps QueryCancel */ ImmediateInterruptOK = false; /* not idle anymore */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* As in quickdie, don't risk sending to client during auth */ if (ClientAuthInProgress && whereToSendOutput == DestRemote) whereToSendOutput = DestNone; @@ -2874,8 +2852,6 @@ ProcessInterrupts(void) { QueryCancelPending = false; /* lost connection trumps QueryCancel */ ImmediateInterruptOK = false; /* not idle anymore */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* don't send to client, we already know the connection to be dead. */ whereToSendOutput = DestNone; ereport(FATAL, @@ -2888,8 +2864,6 @@ ProcessInterrupts(void) if (ClientAuthInProgress) { ImmediateInterruptOK = false; /* not idle anymore */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* As in quickdie, don't risk sending to client during auth */ if (whereToSendOutput == DestRemote) whereToSendOutput = DestNone; @@ -2906,8 +2880,6 @@ ProcessInterrupts(void) { ImmediateInterruptOK = false; /* not idle anymore */ (void) get_timeout_indicator(STATEMENT_TIMEOUT, true); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_LOCK_NOT_AVAILABLE), errmsg("canceling statement due to lock timeout"))); @@ -2915,8 +2887,6 @@ ProcessInterrupts(void) if (get_timeout_indicator(STATEMENT_TIMEOUT, true)) { ImmediateInterruptOK = false; /* not idle anymore */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling statement due to statement timeout"))); @@ -2924,8 +2894,6 @@ ProcessInterrupts(void) if (IsAutoVacuumWorkerProcess()) { ImmediateInterruptOK = false; /* not idle anymore */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling autovacuum task"))); @@ -2934,8 +2902,6 @@ ProcessInterrupts(void) { ImmediateInterruptOK = false; /* not idle anymore */ RecoveryConflictPending = false; - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); pgstat_report_recovery_conflict(RecoveryConflictReason); if (DoingCommandRead) ereport(FATAL, @@ -2959,13 +2925,12 @@ ProcessInterrupts(void) if (!DoingCommandRead) { ImmediateInterruptOK = false; /* not idle anymore */ - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling statement due to user request"))); } } + /* If we get here, do nothing (probably, QueryCancelPending was reset) */ } @@ -3853,13 +3818,9 @@ PostgresMain(int argc, char *argv[], QueryCancelPending = false; /* second to avoid race condition */ /* - * Turn off these interrupts too. This is only needed here and not in - * other exception-catching places since these interrupts are only - * enabled while we wait for client input. + * Not reading from the client anymore. */ DoingCommandRead = false; - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* Make sure libpq is in a good state */ pq_comm_reset(); diff --git a/src/include/commands/async.h b/src/include/commands/async.h index 0650e65..520c17b 100644 --- a/src/include/commands/async.h +++ b/src/include/commands/async.h @@ -13,6 +13,8 @@ #ifndef ASYNC_H #define ASYNC_H +#include + #include "fmgr.h" /* @@ -21,6 +23,7 @@ #define NUM_ASYNC_BUFFERS 8 extern bool Trace_notify; +extern volatile sig_atomic_t notifyInterruptPending; extern Size AsyncShmemSize(void); extern void AsyncShmemInit(void); @@ -48,12 +51,7 @@ extern void ProcessCompletedNotifies(void); /* signal handler for inbound notifies (PROCSIG_NOTIFY_INTERRUPT) */ extern void HandleNotifyInterrupt(void); -/* - * enable/disable processing of inbound notifies directly from signal handler. - * The enable routine first performs processing of any inbound notifies that - * have occurred since the last disable. - */ -extern void EnableNotifyInterrupt(void); -extern bool DisableNotifyInterrupt(void); +/* process interrupts */ +extern void ProcessNotifyInterrupt(void); #endif /* ASYNC_H */ diff --git a/src/include/storage/sinval.h b/src/include/storage/sinval.h index 812ea95..13cd16e 100644 --- a/src/include/storage/sinval.h +++ b/src/include/storage/sinval.h @@ -14,8 +14,9 @@ #ifndef SINVAL_H #define SINVAL_H -#include "storage/relfilenode.h" +#include +#include "storage/relfilenode.h" /* * We support several types of shared-invalidation messages: @@ -123,6 +124,7 @@ typedef union /* Counter of messages processed; don't worry about overflow. */ extern uint64 SharedInvalidMessageCounter; +extern volatile sig_atomic_t catchupInterruptPending; extern void SendSharedInvalidMessages(const SharedInvalidationMessage *msgs, int n); @@ -138,8 +140,7 @@ extern void HandleCatchupInterrupt(void); * The enable routine first performs processing of any catchup events that * have occurred since the last disable. */ -extern void EnableCatchupInterrupt(void); -extern bool DisableCatchupInterrupt(void); +extern void ProcessCatchupInterrupt(void); extern int xactGetCommittedInvalidationMessages(SharedInvalidationMessage **msgs, bool *RelcacheInitFileInval); diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 60f7532..e4a1a7d 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -67,8 +67,8 @@ extern void StatementCancelHandler(SIGNAL_ARGS); extern void FloatExceptionHandler(SIGNAL_ARGS) __attribute__((noreturn)); extern void RecoveryConflictInterrupt(ProcSignalReason reason); /* called from SIGUSR1 * handler */ -extern void prepare_for_client_read(void); -extern void client_read_ended(void); +extern void ProcessClientReadInterrupt(void); + extern void process_postgres_switches(int argc, char *argv[], GucContext ctx, const char **dbname); extern void PostgresMain(int argc, char *argv[], -- 1.8.3.251.g1462b67