Hi, I wonder if we really need signals to implement interrupts. Given that they are non-preemptive/cooperative (work happens at the next CFI()), why not just use shared memory flags and latches? That skips a bunch of code, global variables and scary warnings about programming in signal handlers.
I sketched out some code to try that a few months back, while speculating about bite-sized subproblems that would come up if each backend is, one day, a thread. There are several other conditions that are also handled by CHECK_FOR_INTERRUPTS(), but are not triggered by other backends sending signals, or are set by other signal handlers (SIGALRM, SIGQUIT). One idea is to convert those into "procsignals" too, for consistency. In the attached, they can be set (ie by the same backend) with ProcSignalRaise(), but it's possible that in future we might have a reason for another backend to set them too, so it seems like a good idea to have a single system, effectively merging the concepts of "procsignals" and "interrupts". There are still a few more ad hoc (non-ProcSignal) uses of SIGUSR1 in the tree. For one thing, we don't allow the postmaster to set latches; if we gave up on that rule, we wouldn't need the bgworker please-signal-me thing. Also the logical replication launcher does the same sort of thing for no apparent reason. Changed in the attached -- mainly so I could demonstrate that check-world passes with SIGUSR1 ignored. The attached is only experiment grade code: in particular, I didn't quite untangle the recovery conflict flags properly. It's also doing function calls where some kind of fast inlined magic is probably required, and I probably have a few other details wrong, but I figured it was good enough to demonstrate the concept.
From aa330cf1ecf8665b6fa73caf07c01da0fceeb770 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Sat, 5 Jun 2021 17:13:11 +1200 Subject: [PATCH] WIP: Refactor procsignals and interrupts. Previously, "procsignals" were implemented with shared memory flags and a SIGUSR1 handler that would read shared memory "reason" flags an set process-local "interrupt" flag variables and then set the process latch for race-free wakeup. The next call to CHECK_FOR_INTERRUPTS() would see the interrupt flags and perform the relevant action. Instead, have the sender of the procsignal set the latch directly, have CHECK_FOR_INTERRUPTS() read the "reason" flags directly from shared memory, and get rid of all the global variables. There are several other conditions that also handled by CHECK_FOR_INTERRUPTS(), but are not generated by other backends sending signals, or are set by other signal handlers; these include interrupts triggered by timers and socket events, and are made into "procsignals" too, for consistency. These can be set (ie by the same backend) with ProcSignalRaise(), but it's possible that in future we might have a reason for another backend to set them too, so it seems like a good idea to have a single system, effectively merging the concept of "procsignals" and "interrupts". Also replace some other users of SIGUSR1 that could just as easily use latches, if we gave up the rule that the postmaster can't touch latches. XXX The timeout stuff needs more work XXX The recovery conflict stuff needs more work XXX The new CFI needs to be inline --- contrib/pg_prewarm/autoprewarm.c | 2 +- src/backend/access/transam/parallel.c | 17 +-- src/backend/commands/async.c | 43 +----- src/backend/commands/vacuum.c | 2 +- src/backend/libpq/pqcomm.c | 3 +- src/backend/postmaster/autovacuum.c | 14 +- src/backend/postmaster/bgworker.c | 20 ++- src/backend/postmaster/bgwriter.c | 2 +- src/backend/postmaster/checkpointer.c | 6 +- src/backend/postmaster/interrupt.c | 2 +- src/backend/postmaster/pgarch.c | 4 +- src/backend/postmaster/startup.c | 4 +- src/backend/postmaster/walwriter.c | 4 +- src/backend/regex/regcomp.c | 4 +- src/backend/replication/logical/launcher.c | 20 +-- src/backend/replication/syncrep.c | 7 +- src/backend/replication/walreceiver.c | 2 +- src/backend/replication/walsender.c | 2 +- src/backend/storage/buffer/bufmgr.c | 4 +- src/backend/storage/ipc/dsm_impl.c | 3 +- src/backend/storage/ipc/ipc.c | 5 +- src/backend/storage/ipc/procsignal.c | 153 +++++++++------------ src/backend/storage/ipc/sinval.c | 44 +----- src/backend/tcop/postgres.c | 90 ++++++------ src/backend/utils/init/globals.c | 9 -- src/backend/utils/init/postinit.c | 9 +- src/backend/utils/mmgr/mcxt.c | 18 --- src/include/access/parallel.h | 1 - src/include/commands/async.h | 4 - src/include/miscadmin.h | 22 +-- src/include/postmaster/bgworker.h | 1 + src/include/storage/procsignal.h | 15 +- src/include/storage/sinval.h | 5 - 33 files changed, 197 insertions(+), 344 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index 0289ea657c..d2662a90c4 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -159,7 +159,7 @@ autoprewarm_main(Datum main_arg) /* Establish signal handlers; once that's done, unblock signals. */ pqsignal(SIGTERM, SignalHandlerForShutdownRequest); pqsignal(SIGHUP, SignalHandlerForConfigReload); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); BackgroundWorkerUnblockSignals(); /* Create (if necessary) and attach to our shared memory area. */ diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index bb1881f573..b4e5597156 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -569,7 +569,7 @@ LaunchParallelWorkers(ParallelContext *pcxt) sprintf(worker.bgw_library_name, "postgres"); sprintf(worker.bgw_function_name, "ParallelWorkerMain"); worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(pcxt->seg)); - worker.bgw_notify_pid = MyProcPid; + worker.bgw_notify_pgprocno = MyProc->pgprocno; /* * Start workers. @@ -992,21 +992,6 @@ ParallelContextActive(void) return !dlist_is_empty(&pcxt_list); } -/* - * Handle receipt of an interrupt indicating a parallel worker message. - * - * Note: this is called within a signal handler! All we can do is set - * a flag that will cause the next CHECK_FOR_INTERRUPTS() to invoke - * HandleParallelMessages(). - */ -void -HandleParallelMessageInterrupt(void) -{ - InterruptPending = true; - ParallelMessagePending = true; - SetLatch(MyLatch); -} - /* * Handle any queued protocol messages received from parallel workers. */ diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 8557008545..3b45f87f69 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -421,15 +421,6 @@ typedef struct NotificationHash static NotificationList *pendingNotifies = NULL; -/* - * 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. - */ -volatile sig_atomic_t notifyInterruptPending = false; - /* True if we've registered an on_shmem_exit cleanup */ static bool unlistenExitRegistered = false; @@ -1690,7 +1681,7 @@ SignalBackends(void) */ if (pid == MyProcPid) { - notifyInterruptPending = true; + ProcSignalRaise(PROCSIG_NOTIFY_INTERRUPT); continue; } @@ -1841,37 +1832,12 @@ AtSubAbort_Notify(void) } } -/* - * HandleNotifyInterrupt - * - * 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) -{ - /* - * Note: this is called by a SIGNAL HANDLER. You must be very wary what - * you do here. - */ - - /* signal that work needs to be done */ - notifyInterruptPending = true; - - /* make sure the event is processed in due course */ - SetLatch(MyLatch); -} - /* * ProcessNotifyInterrupt * - * This is called if we see notifyInterruptPending set, just before + * This is called if we see PROCSIG_NOTIFY_INTERRUPT set, just before * transmitting ReadyForQuery at the end of a frontend command, and * also if a notify signal occurs while reading from the frontend. - * HandleNotifyInterrupt() will cause the read to be interrupted - * via the process's latch, and this routine will get called. * If we are truly idle (ie, *not* inside a transaction block), * process the incoming notifies. * @@ -1886,7 +1852,7 @@ ProcessNotifyInterrupt(bool flush) return; /* not really idle */ /* Loop in case another signal arrives while sending messages */ - while (notifyInterruptPending) + while (ProcSignalConsume(PROCSIG_NOTIFY_INTERRUPT)) ProcessIncomingNotify(flush); } @@ -2236,9 +2202,6 @@ asyncQueueAdvanceTail(void) static void ProcessIncomingNotify(bool flush) { - /* We *must* reset the flag */ - notifyInterruptPending = false; - /* Do nothing else if we aren't actively listening */ if (listenChannels == NIL) return; diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 5c4bc15b44..7707510725 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2153,7 +2153,7 @@ vacuum_delay_point(void) /* Always check for interrupts */ CHECK_FOR_INTERRUPTS(); - if (!VacuumCostActive || InterruptPending) + if (!VacuumCostActive || ProcSignalAnyPending()) return; /* diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index 89a5f901aa..5deb700981 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -1390,8 +1390,7 @@ internal_flush(void) * the connection. */ PqSendStart = PqSendPointer = 0; - ClientConnectionLost = 1; - InterruptPending = 1; + ProcSignalRaise(PROCSIG_CONNECTION_LOST); return EOF; } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 96332320a7..2e09b76fcf 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -463,7 +463,7 @@ AutoVacLauncherMain(int argc, char *argv[]) InitializeTimeouts(); /* establishes SIGALRM handler */ pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, avl_sigusr2_handler); pqsignal(SIGFPE, FloatExceptionHandler); pqsignal(SIGCHLD, SIG_DFL); @@ -517,7 +517,7 @@ AutoVacLauncherMain(int argc, char *argv[]) /* Forget any pending QueryCancel or timeout request */ disable_all_timeouts(false); - QueryCancelPending = false; /* second to avoid race condition */ + ProcSignalClear(PROCSIG_QUERY_CANCEL); /* second to avoid race condition */ /* Report the error to the server log */ EmitErrorReport(); @@ -833,11 +833,11 @@ HandleAutoVacLauncherInterrupts(void) } /* Process barrier events */ - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); /* Perform logging of memory contexts of this process */ - if (LogMemoryContextPending) + if (ProcSignalPending(PROCSIG_LOG_MEMORY_CONTEXT)) ProcessLogMemoryContextInterrupt(); /* Process sinval catchup interrupts that happened while sleeping */ @@ -1545,7 +1545,7 @@ AutoVacWorkerMain(int argc, char *argv[]) InitializeTimeouts(); /* establishes SIGALRM handler */ pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, SIG_IGN); pqsignal(SIGFPE, FloatExceptionHandler); pqsignal(SIGCHLD, SIG_DFL); @@ -2499,7 +2499,7 @@ do_autovacuum(void) * current table (we're done with it, so it would make no sense to * cancel at this point.) */ - QueryCancelPending = false; + ProcSignalClear(PROCSIG_QUERY_CANCEL); } PG_CATCH(); { @@ -2696,7 +2696,7 @@ perform_work_item(AutoVacuumWorkItem *workitem) * (we're done with it, so it would make no sense to cancel at this * point.) */ - QueryCancelPending = false; + ProcSignalClear(PROCSIG_QUERY_CANCEL); } PG_CATCH(); { diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index c05f500639..ac57ad170c 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -190,6 +190,7 @@ BackgroundWorkerShmemInit(void) slot->generation = 0; rw->rw_shmem_slot = slotno; rw->rw_worker.bgw_notify_pid = 0; /* might be reinit after crash */ + rw->rw_worker.bgw_notify_pgprocno = 0; memcpy(&slot->worker, &rw->rw_worker, sizeof(BackgroundWorker)); ++slotno; } @@ -318,6 +319,7 @@ BackgroundWorkerStateChange(bool allow_new_workers) if (slot->terminate) { int notify_pid; + int notify_pgprocno; /* * We need a memory barrier here to make sure that the load of @@ -325,6 +327,7 @@ BackgroundWorkerStateChange(bool allow_new_workers) * complete before the store to in_use. */ notify_pid = slot->worker.bgw_notify_pid; + notify_pgprocno = slot->worker.bgw_notify_pgprocno; if ((slot->worker.bgw_flags & BGWORKER_CLASS_PARALLEL) != 0) BackgroundWorkerData->parallel_terminate_count++; slot->pid = 0; @@ -334,6 +337,8 @@ BackgroundWorkerStateChange(bool allow_new_workers) if (notify_pid != 0) kill(notify_pid, SIGUSR1); + if (notify_pgprocno != 0) + SetLatch(&ProcGlobal->allProcs[notify_pgprocno].procLatch); continue; } @@ -393,6 +398,7 @@ BackgroundWorkerStateChange(bool allow_new_workers) (long) rw->rw_worker.bgw_notify_pid); rw->rw_worker.bgw_notify_pid = 0; } + rw->rw_worker.bgw_notify_pgprocno = slot->worker.bgw_notify_pgprocno; /* Initialize postmaster bookkeeping. */ rw->rw_backend = NULL; @@ -468,6 +474,8 @@ ReportBackgroundWorkerPID(RegisteredBgWorker *rw) if (rw->rw_worker.bgw_notify_pid != 0) kill(rw->rw_worker.bgw_notify_pid, SIGUSR1); + if (rw->rw_worker.bgw_notify_pgprocno != 0) + SetLatch(&ProcGlobal->allProcs[rw->rw_worker.bgw_notify_pgprocno].procLatch); } /* @@ -482,6 +490,7 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur) RegisteredBgWorker *rw; BackgroundWorkerSlot *slot; int notify_pid; + int notify_pgprocno; rw = slist_container(RegisteredBgWorker, rw_lnode, cur->cur); @@ -489,6 +498,7 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur) slot = &BackgroundWorkerData->slot[rw->rw_shmem_slot]; slot->pid = rw->rw_pid; notify_pid = rw->rw_worker.bgw_notify_pid; + notify_pgprocno = rw->rw_worker.bgw_notify_pgprocno; /* * If this worker is slated for deregistration, do that before notifying @@ -503,6 +513,8 @@ ReportBackgroundWorkerExit(slist_mutable_iter *cur) if (notify_pid != 0) kill(notify_pid, SIGUSR1); + if (notify_pgprocno != 0) + SetLatch(&ProcGlobal->allProcs[notify_pgprocno].procLatch); } /* @@ -553,14 +565,18 @@ ForgetUnstartedBackgroundWorkers(void) /* If it's not yet started, and there's someone waiting ... */ if (slot->pid == InvalidPid && - rw->rw_worker.bgw_notify_pid != 0) + (rw->rw_worker.bgw_notify_pid != 0 || + rw->rw_worker.bgw_notify_pgprocno != 0)) { /* ... then zap it, and notify the waiter */ int notify_pid = rw->rw_worker.bgw_notify_pid; + int notify_pgprocno = rw->rw_worker.bgw_notify_pgprocno; ForgetBackgroundWorker(&iter); if (notify_pid != 0) kill(notify_pid, SIGUSR1); + if (notify_pgprocno != 0) + SetLatch(&ProcGlobal->allProcs[notify_pgprocno].procLatch); } } } @@ -618,6 +634,7 @@ ResetBackgroundWorkerCrashTimes(void) * If there was anyone waiting for it, they're history. */ rw->rw_worker.bgw_notify_pid = 0; + rw->rw_worker.bgw_notify_pgprocno = 0; } } } @@ -767,7 +784,6 @@ StartBackgroundWorker(void) * SIGINT is used to signal canceling the current action */ pqsignal(SIGINT, StatementCancelHandler); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); pqsignal(SIGFPE, FloatExceptionHandler); /* XXX Any other handlers needed here? */ diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index 5584f4bc24..3879acfa6c 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -104,7 +104,7 @@ BackgroundWriterMain(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, SIG_IGN); /* diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index be7366379d..9f6d30fc52 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -198,7 +198,7 @@ CheckpointerMain(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, SignalHandlerForShutdownRequest); /* @@ -537,7 +537,7 @@ CheckpointerMain(void) static void HandleCheckpointerInterrupts(void) { - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); if (ConfigReloadPending) @@ -739,7 +739,7 @@ CheckpointWriteDelay(int flags, double progress) } /* Check for barrier events. */ - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); } diff --git a/src/backend/postmaster/interrupt.c b/src/backend/postmaster/interrupt.c index dd9136a942..e7d391b8c4 100644 --- a/src/backend/postmaster/interrupt.c +++ b/src/backend/postmaster/interrupt.c @@ -32,7 +32,7 @@ volatile sig_atomic_t ShutdownRequestPending = false; void HandleMainLoopInterrupts(void) { - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); if (ConfigReloadPending) diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 74a7d7c4d0..0b19b92459 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -177,7 +177,7 @@ PgArchiverMain(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, pgarch_waken_stop); /* Reset some signals that are accepted by postmaster but not here */ @@ -707,7 +707,7 @@ pgarch_die(int code, Datum arg) static void HandlePgArchInterrupts(void) { - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); if (ConfigReloadPending) diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index 0f4f00d689..43ca44e3b7 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -182,7 +182,7 @@ HandleStartupProcInterrupts(void) exit(1); /* Process barrier events */ - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); } @@ -219,7 +219,7 @@ StartupProcessMain(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ InitializeTimeouts(); /* establishes SIGALRM handler */ pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, StartupProcTriggerHandler); /* diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 626fae8454..7f29202ae3 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -107,7 +107,7 @@ WalWriterMain(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, SIG_IGN); /* not used */ /* @@ -283,7 +283,7 @@ WalWriterMain(void) static void HandleWalWriterInterrupts(void) { - if (ProcSignalBarrierPending) + if (ProcSignalPending(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); if (ConfigReloadPending) diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c index 473738040b..dbba85bf42 100644 --- a/src/backend/regex/regcomp.c +++ b/src/backend/regex/regcomp.c @@ -2437,7 +2437,9 @@ rfree(regex_t *re) static int rcancelrequested(void) { - return InterruptPending && (QueryCancelPending || ProcDiePending); + return ProcSignalAnyPending() && + (ProcSignalPending(PROCSIG_QUERY_CANCEL) || + ProcSignalPending(PROCSIG_DIE)); } /* diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c index 3fb4caa803..bd4460ea72 100644 --- a/src/backend/replication/logical/launcher.c +++ b/src/backend/replication/logical/launcher.c @@ -59,7 +59,7 @@ LogicalRepWorker *MyLogicalRepWorker = NULL; typedef struct LogicalRepCtxStruct { /* Supervisor process. */ - pid_t launcher_pid; + int launcher_pgprocno; /* Background workers. */ LogicalRepWorker workers[FLEXIBLE_ARRAY_MEMBER]; @@ -407,7 +407,7 @@ retry: snprintf(bgw.bgw_type, BGW_MAXLEN, "logical replication worker"); bgw.bgw_restart_time = BGW_NEVER_RESTART; - bgw.bgw_notify_pid = MyProcPid; + bgw.bgw_notify_pgprocno = MyProc->pgprocno; bgw.bgw_main_arg = Int32GetDatum(slot); if (!RegisterDynamicBackgroundWorker(&bgw, &bgw_handle)) @@ -632,7 +632,7 @@ logicalrep_worker_cleanup(LogicalRepWorker *worker) static void logicalrep_launcher_onexit(int code, Datum arg) { - LogicalRepCtx->launcher_pid = 0; + LogicalRepCtx->launcher_pgprocno = 0; } /* @@ -722,7 +722,7 @@ ApplyLauncherRegister(void) snprintf(bgw.bgw_type, BGW_MAXLEN, "logical replication launcher"); bgw.bgw_restart_time = 5; - bgw.bgw_notify_pid = 0; + bgw.bgw_notify_pgprocno = 0; bgw.bgw_main_arg = (Datum) 0; RegisterBackgroundWorker(&bgw); @@ -791,8 +791,10 @@ ApplyLauncherWakeupAtCommit(void) static void ApplyLauncherWakeup(void) { - if (LogicalRepCtx->launcher_pid != 0) - kill(LogicalRepCtx->launcher_pid, SIGUSR1); + int pgprocno; + + if ((pgprocno = LogicalRepCtx->launcher_pgprocno) != 0) + SetLatch(&ProcGlobal->allProcs[pgprocno].procLatch); } /* @@ -808,8 +810,8 @@ ApplyLauncherMain(Datum main_arg) before_shmem_exit(logicalrep_launcher_onexit, (Datum) 0); - Assert(LogicalRepCtx->launcher_pid == 0); - LogicalRepCtx->launcher_pid = MyProcPid; + Assert(LogicalRepCtx->launcher_pgprocno == 0); + LogicalRepCtx->launcher_pgprocno = MyProc->pgprocno; /* Establish signal handlers. */ pqsignal(SIGHUP, SignalHandlerForConfigReload); @@ -917,7 +919,7 @@ ApplyLauncherMain(Datum main_arg) bool IsLogicalLauncher(void) { - return LogicalRepCtx->launcher_pid == MyProcPid; + return LogicalRepCtx->launcher_pgprocno == MyProc->pgprocno; } /* diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index bdbc9ef844..fe23de7f55 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -261,7 +261,7 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) * We do NOT reset ProcDiePending, so that the process will die after * the commit is cleaned up. */ - if (ProcDiePending) + if (ProcSignalPending(PROCSIG_DIE)) { ereport(WARNING, (errcode(ERRCODE_ADMIN_SHUTDOWN), @@ -278,9 +278,8 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) * altogether is not helpful, so we just terminate the wait with a * suitable warning. */ - if (QueryCancelPending) + if (ProcSignalConsume(PROCSIG_QUERY_CANCEL)) { - QueryCancelPending = false; ereport(WARNING, (errmsg("canceling wait for synchronous replication due to user request"), errdetail("The transaction has already committed locally, but might not have been replicated to the standby."))); @@ -301,7 +300,7 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) */ if (rc & WL_POSTMASTER_DEATH) { - ProcDiePending = true; + ProcSignalRaise(PROCSIG_DIE); whereToSendOutput = DestNone; SyncRepCancelWait(); break; diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index b90e5ca98e..ba9fc59509 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -265,7 +265,7 @@ WalReceiverMain(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, SIG_IGN); /* Reset some signals that are accepted by postmaster but not here */ diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index b811a5c0ef..84610399f2 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -3067,7 +3067,7 @@ WalSndSignals(void) /* SIGQUIT handler was already set up by InitPostmasterChild */ InitializeTimeouts(); /* establishes SIGALRM handler */ pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); pqsignal(SIGUSR2, WalSndLastCycleHandler); /* request a last cycle and * shutdown */ diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 08ebabfe96..043ef2e371 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2001,7 +2001,7 @@ BufferSync(int flags) UnlockBufHdr(bufHdr, buf_state); /* Check for barrier events in case NBuffers is large. */ - if (ProcSignalBarrierPending) + if (ProcSignalConsume(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); } @@ -2082,7 +2082,7 @@ BufferSync(int flags) s->num_to_scan++; /* Check for barrier events. */ - if (ProcSignalBarrierPending) + if (ProcSignalConsume(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); } diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index f7e292981e..10dac87fe6 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -385,7 +385,8 @@ dsm_impl_posix_resize(int fd, off_t size) do { rc = posix_fallocate(fd, 0, size); - } while (rc == EINTR && !(ProcDiePending || QueryCancelPending)); + } while (rc == EINTR && !(ProcSignalPending(PROCSIG_DIE) || + ProcSignalPending(PROCSIG_QUERY_CANCEL))); pgstat_report_wait_end(); /* diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c index 4045d7d68a..68539f6448 100644 --- a/src/backend/storage/ipc/ipc.c +++ b/src/backend/storage/ipc/ipc.c @@ -171,9 +171,8 @@ proc_exit_prepare(int code) * close up shop already. Note that the signal handlers will not set * these flags again, now that proc_exit_inprogress is set. */ - InterruptPending = false; - ProcDiePending = false; - QueryCancelPending = false; + ProcSignalClear(PROCSIG_DIE); + ProcSignalClear(PROCSIG_QUERY_CANCEL); InterruptHoldoffCount = 1; CritSectionCount = 0; diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index defb75aa26..f385df58b8 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -62,7 +62,9 @@ typedef struct { volatile pid_t pss_pid; - volatile sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS]; + volatile int pss_pgprocno; + volatile bool pss_signaled; /* formerly InterruptPending */ + volatile bool pss_signalFlags[NUM_PROCSIGNALS]; pg_atomic_uint64 pss_barrierGeneration; pg_atomic_uint32 pss_barrierCheckMask; ConditionVariable pss_barrierCV; @@ -98,7 +100,6 @@ typedef struct static ProcSignalHeader *ProcSignal = NULL; static ProcSignalSlot *MyProcSignalSlot = NULL; -static bool CheckProcSignal(ProcSignalReason reason); static void CleanupProcSignalState(int status, Datum arg); static void ResetProcSignalBarrierBits(uint32 flags); static bool ProcessBarrierPlaceholder(void); @@ -142,7 +143,10 @@ ProcSignalShmemInit(void) ProcSignalSlot *slot = &ProcSignal->psh_slot[i]; slot->pss_pid = 0; - MemSet(slot->pss_signalFlags, 0, sizeof(slot->pss_signalFlags)); + slot->pss_pgprocno = 0; + slot->pss_signaled = false; + for (int j = 0; j < NUM_PROCSIGNALS; ++j) + slot->pss_signalFlags[j] = false; pg_atomic_init_u64(&slot->pss_barrierGeneration, PG_UINT64_MAX); pg_atomic_init_u32(&slot->pss_barrierCheckMask, 0); ConditionVariableInit(&slot->pss_barrierCV); @@ -173,7 +177,9 @@ ProcSignalInit(int pss_idx) MyProcPid, pss_idx); /* Clear out any leftover signal reasons */ - MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t)); + slot->pss_signaled = false; + for (int i = 0; i < NUM_PROCSIGNALS; ++i) + slot->pss_signalFlags[i] = false; /* * Initialize barrier state. Since we're a brand-new process, there @@ -192,8 +198,9 @@ ProcSignalInit(int pss_idx) pg_atomic_write_u64(&slot->pss_barrierGeneration, barrier_generation); pg_memory_barrier(); - /* Mark slot with my PID */ + /* Mark slot with my procno, so my latch will be set for proc signals */ slot->pss_pid = MyProcPid; + slot->pss_pgprocno = MyProc->pgprocno; /* Remember slot location for CheckProcSignal */ MyProcSignalSlot = slot; @@ -225,14 +232,14 @@ CleanupProcSignalState(int status, Datum arg) MyProcSignalSlot = NULL; /* sanity check */ - if (slot->pss_pid != MyProcPid) + if (slot->pss_pgprocno != MyProc->pgprocno) { /* * don't ERROR here. We're exiting anyway, and don't want to get into * infinite loop trying to exit */ elog(LOG, "process %d releasing ProcSignal slot %d, but it contains %d", - MyProcPid, pss_idx, (int) slot->pss_pid); + MyProcPid, pss_idx, slot->pss_pgprocno); return; /* XXX better to zero the slot anyway? */ } @@ -244,6 +251,7 @@ CleanupProcSignalState(int status, Datum arg) ConditionVariableBroadcast(&slot->pss_barrierCV); slot->pss_pid = 0; + slot->pss_pgprocno = 0; } /* @@ -276,10 +284,11 @@ SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId) */ if (slot->pss_pid == pid) { - /* Atomically set the proper flag */ slot->pss_signalFlags[reason] = true; - /* Send signal */ - return kill(pid, SIGUSR1); + pg_write_barrier(); + slot->pss_signaled = true; + SetLatch(&ProcGlobal->allProcs[slot->pss_pgprocno].procLatch); + return 0; } } else @@ -299,11 +308,11 @@ SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId) if (slot->pss_pid == pid) { /* the above note about race conditions applies here too */ - - /* Atomically set the proper flag */ slot->pss_signalFlags[reason] = true; - /* Send signal */ - return kill(pid, SIGUSR1); + pg_write_barrier(); + slot->pss_signaled = true; + SetLatch(&ProcGlobal->allProcs[slot->pss_pgprocno].procLatch); + return 0; } } } @@ -377,7 +386,9 @@ EmitProcSignalBarrier(ProcSignalBarrierType type) { /* see SendProcSignal for details */ slot->pss_signalFlags[PROCSIG_BARRIER] = true; - kill(pid, SIGUSR1); + pg_write_barrier(); + slot->pss_signaled = true; + SetLatch(&ProcGlobal->allProcs[slot->pss_pgprocno].procLatch); } } @@ -424,23 +435,6 @@ WaitForProcSignalBarrier(uint64 generation) pg_memory_barrier(); } -/* - * Handle receipt of an interrupt indicating a global barrier event. - * - * All the actual work is deferred to ProcessProcSignalBarrier(), because we - * cannot safely access the barrier generation inside the signal handler as - * 64bit atomics might use spinlock based emulation, even for reads. As this - * routine only gets called when PROCSIG_BARRIER is sent that won't cause a - * lot of unnecessary work. - */ -static void -HandleProcSignalBarrierInterrupt(void) -{ - InterruptPending = true; - ProcSignalBarrierPending = true; - /* latch will be set by procsignal_sigusr1_handler */ -} - /* * Perform global barrier related interrupt checking. * @@ -458,11 +452,6 @@ ProcessProcSignalBarrier(void) Assert(MyProcSignalSlot); - /* Exit quickly if there's no work to do. */ - if (!ProcSignalBarrierPending) - return; - ProcSignalBarrierPending = false; - /* * It's not unlikely to process multiple barriers at once, before the * signals for all the barriers have arrived. To avoid unnecessary work in @@ -590,8 +579,7 @@ static void ResetProcSignalBarrierBits(uint32 flags) { pg_atomic_fetch_or_u32(&MyProcSignalSlot->pss_barrierCheckMask, flags); - ProcSignalBarrierPending = true; - InterruptPending = true; + ProcSignalRaise(PROCSIG_BARRIER); } static bool @@ -612,13 +600,28 @@ ProcessBarrierPlaceholder(void) return true; } -/* - * CheckProcSignal - check to see if a particular reason has been - * signaled, and clear the signal flag. Should be called after receiving - * SIGUSR1. - */ -static bool -CheckProcSignal(ProcSignalReason reason) +void +ProcSignalRaise(ProcSignalReason reason) +{ + volatile ProcSignalSlot *slot = MyProcSignalSlot; + + if (slot) + { + slot->pss_signalFlags[reason] = true; + slot->pss_signaled = true; + } +} + +bool +ProcSignalPending(ProcSignalReason reason) +{ + volatile ProcSignalSlot *slot = MyProcSignalSlot; + + return slot && slot->pss_signalFlags[reason]; +} + +bool +ProcSignalConsume(ProcSignalReason reason) { volatile ProcSignalSlot *slot = MyProcSignalSlot; @@ -635,51 +638,29 @@ CheckProcSignal(ProcSignalReason reason) return false; } -/* - * procsignal_sigusr1_handler - handle SIGUSR1 signal. - */ void -procsignal_sigusr1_handler(SIGNAL_ARGS) +ProcSignalClear(ProcSignalReason reason) { - int save_errno = errno; - - if (CheckProcSignal(PROCSIG_CATCHUP_INTERRUPT)) - HandleCatchupInterrupt(); - - if (CheckProcSignal(PROCSIG_NOTIFY_INTERRUPT)) - HandleNotifyInterrupt(); - - if (CheckProcSignal(PROCSIG_PARALLEL_MESSAGE)) - HandleParallelMessageInterrupt(); - - if (CheckProcSignal(PROCSIG_WALSND_INIT_STOPPING)) - HandleWalSndInitStopping(); - - if (CheckProcSignal(PROCSIG_BARRIER)) - HandleProcSignalBarrierInterrupt(); - - if (CheckProcSignal(PROCSIG_LOG_MEMORY_CONTEXT)) - HandleLogMemoryContextInterrupt(); - - if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_DATABASE)) - RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_DATABASE); - - if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_TABLESPACE)) - RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_TABLESPACE); - - if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_LOCK)) - RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_LOCK); + volatile ProcSignalSlot *slot = MyProcSignalSlot; - if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT)) - RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_SNAPSHOT); + if (slot) + slot->pss_signalFlags[reason] = false; +} - if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK)) - RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK); +bool +ProcSignalAnyPending(void) +{ + volatile ProcSignalSlot *slot = MyProcSignalSlot; - if (CheckProcSignal(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN)) - RecoveryConflictInterrupt(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN); + /* XXX make this static inline? */ + /* XXX point to a dummy entry instead of using NULL to avoid a branch */ + return slot && slot->pss_signaled; +} - SetLatch(MyLatch); +void +ProcSignalClearAnyPending(void) +{ + volatile ProcSignalSlot *slot = MyProcSignalSlot; - errno = save_errno; + slot->pss_signaled = false; } diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index f585d63e5c..3c174f96dc 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/src/backend/storage/ipc/sinval.c @@ -26,21 +26,6 @@ uint64 SharedInvalidMessageCounter; -/* - * 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. - * - * The signal handler will set an 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(). - */ -volatile sig_atomic_t catchupInterruptPending = false; - - /* * SendSharedInvalidMessages * Add shared-cache-invalidation message(s) to the global SI message queue. @@ -134,38 +119,13 @@ ReceiveSharedInvalidMessages(void (*invalFunction) (SharedInvalidationMessage *m * catchup signal this way avoids creating spikes in system load for what * should be just a background maintenance activity. */ - if (catchupInterruptPending) + if (ProcSignalConsume(PROCSIG_CATCHUP_INTERRUPT)) { - catchupInterruptPending = false; elog(DEBUG4, "sinval catchup complete, cleaning queue"); SICleanupQueue(false, 0); } } - -/* - * HandleCatchupInterrupt - * - * This is called when PROCSIG_CATCHUP_INTERRUPT is received. - * - * 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 process's latch. - */ -void -HandleCatchupInterrupt(void) -{ - /* - * Note: this is called by a SIGNAL HANDLER. You must be very wary what - * you do here. - */ - - catchupInterruptPending = true; - - /* make sure the event is processed in due course */ - SetLatch(MyLatch); -} - /* * ProcessCatchupInterrupt * @@ -175,7 +135,7 @@ HandleCatchupInterrupt(void) void ProcessCatchupInterrupt(void) { - while (catchupInterruptPending) + while (ProcSignalPending(PROCSIG_CATCHUP_INTERRUPT)) { /* * What we need to do here is cause ReceiveSharedInvalidMessages() to diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 0775abe35d..bfbef80e32 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -499,14 +499,14 @@ ProcessClientReadInterrupt(bool blocked) CHECK_FOR_INTERRUPTS(); /* Process sinval catchup interrupts, if any */ - if (catchupInterruptPending) + if (ProcSignalPending(PROCSIG_CATCHUP_INTERRUPT)) ProcessCatchupInterrupt(); /* Process notify interrupts, if any */ - if (notifyInterruptPending) + if (ProcSignalPending(PROCSIG_NOTIFY_INTERRUPT)) ProcessNotifyInterrupt(true); } - else if (ProcDiePending) + else if (ProcSignalPending(PROCSIG_DIE)) { /* * We're dying. If there is no data available to read, then it's safe @@ -539,7 +539,7 @@ ProcessClientWriteInterrupt(bool blocked) { int save_errno = errno; - if (ProcDiePending) + if (ProcSignalPending(PROCSIG_DIE)) { /* * We're dying. If it's not possible to write, then we should handle @@ -2918,10 +2918,7 @@ die(SIGNAL_ARGS) /* Don't joggle the elbow of proc_exit */ if (!proc_exit_inprogress) - { - InterruptPending = true; - ProcDiePending = true; - } + ProcSignalRaise(PROCSIG_DIE); /* for the statistics collector */ pgStatSessionEndCause = DISCONNECT_KILLED; @@ -2954,10 +2951,7 @@ StatementCancelHandler(SIGNAL_ARGS) * Don't joggle the elbow of proc_exit */ if (!proc_exit_inprogress) - { - InterruptPending = true; - QueryCancelPending = true; - } + ProcSignalRaise(PROCSIG_QUERY_CANCEL); /* If we're still here, waken anything waiting on the process latch */ SetLatch(MyLatch); @@ -3070,9 +3064,8 @@ RecoveryConflictInterrupt(ProcSignalReason reason) if (IsAbortedTransactionBlockState()) return; - RecoveryConflictPending = true; - QueryCancelPending = true; - InterruptPending = true; + ProcSignalRaise(PROCSIG_RECOVERY_CONFLICT); + ProcSignalRaise(PROCSIG_QUERY_CANCEL); break; } @@ -3080,9 +3073,7 @@ RecoveryConflictInterrupt(ProcSignalReason reason) /* FALLTHROUGH */ case PROCSIG_RECOVERY_CONFLICT_DATABASE: - RecoveryConflictPending = true; - ProcDiePending = true; - InterruptPending = true; + ProcSignalRaise(PROCSIG_RECOVERY_CONFLICT_DATABASE); break; default: @@ -3090,7 +3081,9 @@ RecoveryConflictInterrupt(ProcSignalReason reason) (int) reason); } - Assert(RecoveryConflictPending && (QueryCancelPending || ProcDiePending)); + Assert(ProcSignalPending(PROCSIG_RECOVERY_CONFLICT) && + (ProcSignalPending(PROCSIG_QUERY_CANCEL) || + ProcSignalPending(PROCSIG_DIE))); /* * All conflicts apart from database cause dynamic errors where the @@ -3117,7 +3110,7 @@ RecoveryConflictInterrupt(ProcSignalReason reason) * * If an interrupt condition is pending, and it's safe to service it, * then clear the flag and accept the interrupt. Called only when - * InterruptPending is true. + * ProcSignalPending() is true. * * Note: if INTERRUPTS_CAN_BE_PROCESSED() is true, then ProcessInterrupts * is guaranteed to clear the InterruptPending flag before returning. @@ -3131,12 +3124,13 @@ ProcessInterrupts(void) /* OK to accept any interrupts now? */ if (InterruptHoldoffCount != 0 || CritSectionCount != 0) return; - InterruptPending = false; + ProcSignalClearAnyPending(); + + pg_read_barrier(); - if (ProcDiePending) + if (ProcSignalConsume(PROCSIG_DIE)) { - ProcDiePending = false; - QueryCancelPending = false; /* ProcDie trumps QueryCancel */ + ProcSignalClear(PROCSIG_QUERY_CANCEL); /* ProcDie trumps QueryCancel */ LockErrorCleanup(); /* As in quickdie, don't risk sending to client during auth */ if (ClientAuthInProgress && whereToSendOutput == DestRemote) @@ -3193,10 +3187,8 @@ ProcessInterrupts(void) errmsg("terminating connection due to administrator command"))); } - if (CheckClientConnectionPending) + if (ProcSignalConsume(PROCSIG_CHECK_CONNECTION_TIMEOUT)) { - CheckClientConnectionPending = false; - /* * Check for lost connection and re-arm, if still configured, but not * if we've arrived back at DoingCommandRead state. We don't want to @@ -3206,16 +3198,16 @@ ProcessInterrupts(void) if (!DoingCommandRead && client_connection_check_interval > 0) { if (!pq_check_connection()) - ClientConnectionLost = true; + ProcSignalRaise(PROCSIG_CONNECTION_LOST); else enable_timeout_after(CLIENT_CONNECTION_CHECK_TIMEOUT, client_connection_check_interval); } } - if (ClientConnectionLost) + if (ProcSignalConsume(PROCSIG_CONNECTION_LOST)) { - QueryCancelPending = false; /* lost connection trumps QueryCancel */ + ProcSignalClear(PROCSIG_QUERY_CANCEL); /* lost connection trumps */ LockErrorCleanup(); /* don't send to client, we already know the connection to be dead. */ whereToSendOutput = DestNone; @@ -3230,10 +3222,10 @@ ProcessInterrupts(void) * preventing recovery from making progress. Terminate the connection to * dislodge it. */ - if (RecoveryConflictPending && DoingCommandRead) + if (ProcSignalPending(PROCSIG_RECOVERY_CONFLICT) && DoingCommandRead) { - QueryCancelPending = false; /* this trumps QueryCancel */ - RecoveryConflictPending = false; + ProcSignalClear(PROCSIG_QUERY_CANCEL); /* this trumps QueryCancel */ + ProcSignalClear(PROCSIG_RECOVERY_CONFLICT); LockErrorCleanup(); pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(FATAL, @@ -3250,7 +3242,8 @@ ProcessInterrupts(void) * interrupts are OK, because we won't read any further messages from the * client in that case.) */ - if (QueryCancelPending && QueryCancelHoldoffCount != 0) + if (ProcSignalPending(PROCSIG_QUERY_CANCEL) && + QueryCancelHoldoffCount != 0) { /* * Re-arm InterruptPending so that we process the cancel request as @@ -3260,15 +3253,13 @@ ProcessInterrupts(void) * meaning that this code also creates opportunities for other bugs to * appear.) */ - InterruptPending = true; + ProcSignalRaise(PROCSIG_QUERY_CANCEL); } - else if (QueryCancelPending) + else if (ProcSignalConsume(PROCSIG_QUERY_CANCEL)) { bool lock_timeout_occurred; bool stmt_timeout_occurred; - QueryCancelPending = false; - /* * If LOCK_TIMEOUT and STATEMENT_TIMEOUT indicators are both set, we * need to clear both, so always fetch both. @@ -3332,7 +3323,7 @@ ProcessInterrupts(void) } } - if (IdleInTransactionSessionTimeoutPending) + if (ProcSignalConsume(PROCSIG_IDLE_TRANSACTION_TIMEOUT)) { /* * If the GUC has been reset to zero, ignore the signal. This is @@ -3343,28 +3334,27 @@ ProcessInterrupts(void) ereport(FATAL, (errcode(ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT), errmsg("terminating connection due to idle-in-transaction timeout"))); - else - IdleInTransactionSessionTimeoutPending = false; } - if (IdleSessionTimeoutPending) + if (ProcSignalConsume(PROCSIG_IDLE_SESSION_TIMEOUT)) { /* As above, ignore the signal if the GUC has been reset to zero. */ if (IdleSessionTimeout > 0) ereport(FATAL, (errcode(ERRCODE_IDLE_SESSION_TIMEOUT), errmsg("terminating connection due to idle-session timeout"))); - else - IdleSessionTimeoutPending = false; } - if (ProcSignalBarrierPending) + if (ProcSignalConsume(PROCSIG_BARRIER)) ProcessProcSignalBarrier(); - if (ParallelMessagePending) + if (ProcSignalConsume(PROCSIG_PARALLEL_MESSAGE)) HandleParallelMessages(); - if (LogMemoryContextPending) + if (ProcSignalConsume(PROCSIG_WALSND_INIT_STOPPING)) + HandleWalSndInitStopping(); + + if (ProcSignalConsume(PROCSIG_LOG_MEMORY_CONTEXT)) ProcessLogMemoryContextInterrupt(); } @@ -4070,7 +4060,7 @@ PostgresMain(const char *dbname, const char *username) * midst of output during who-knows-what operation... */ pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, procsignal_sigusr1_handler); + pqsignal(SIGUSR1, SIG_IGN); //procsignal_sigusr1_handler); /* XXX which, see */ pqsignal(SIGUSR2, SIG_IGN); pqsignal(SIGFPE, FloatExceptionHandler); @@ -4228,7 +4218,7 @@ PostgresMain(const char *dbname, const char *username) * forgetting a timeout cancel. */ disable_all_timeouts(false); - QueryCancelPending = false; /* second to avoid race condition */ + ProcSignalClear(PROCSIG_QUERY_CANCEL); /* second to avoid race condition */ /* Not reading from the client anymore. */ DoingCommandRead = false; @@ -4391,7 +4381,7 @@ PostgresMain(const char *dbname, const char *username) * were received during the just-finished transaction, they'll * be seen by the client before ReadyForQuery is. */ - if (notifyInterruptPending) + if (ProcSignalPending(PROCSIG_NOTIFY_INTERRUPT)) ProcessNotifyInterrupt(false); pgstat_report_stat(false); diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 381d9e548d..811a74278b 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -27,15 +27,6 @@ ProtocolVersion FrontendProtocol; -volatile sig_atomic_t InterruptPending = false; -volatile sig_atomic_t QueryCancelPending = false; -volatile sig_atomic_t ProcDiePending = false; -volatile sig_atomic_t CheckClientConnectionPending = false; -volatile sig_atomic_t ClientConnectionLost = false; -volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false; -volatile sig_atomic_t IdleSessionTimeoutPending = false; -volatile sig_atomic_t ProcSignalBarrierPending = false; -volatile sig_atomic_t LogMemoryContextPending = false; volatile uint32 InterruptHoldoffCount = 0; volatile uint32 QueryCancelHoldoffCount = 0; volatile uint32 CritSectionCount = 0; diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 78bc64671e..dae09e5b66 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1224,24 +1224,21 @@ LockTimeoutHandler(void) static void IdleInTransactionSessionTimeoutHandler(void) { - IdleInTransactionSessionTimeoutPending = true; - InterruptPending = true; + ProcSignalRaise(PROCSIG_IDLE_TRANSACTION_TIMEOUT); SetLatch(MyLatch); } static void IdleSessionTimeoutHandler(void) { - IdleSessionTimeoutPending = true; - InterruptPending = true; + ProcSignalRaise(PROCSIG_IDLE_SESSION_TIMEOUT); SetLatch(MyLatch); } static void ClientCheckTimeoutHandler(void) { - CheckClientConnectionPending = true; - InterruptPending = true; + ProcSignalRaise(PROCSIG_CHECK_CONNECTION_TIMEOUT); SetLatch(MyLatch); } diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c index 6919a73280..84c0537440 100644 --- a/src/backend/utils/mmgr/mcxt.c +++ b/src/backend/utils/mmgr/mcxt.c @@ -1012,22 +1012,6 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags) return ret; } -/* - * HandleLogMemoryContextInterrupt - * Handle receipt of an interrupt indicating logging of memory - * contexts. - * - * All the actual work is deferred to ProcessLogMemoryContextInterrupt(), - * because we cannot safely emit a log message inside the signal handler. - */ -void -HandleLogMemoryContextInterrupt(void) -{ - InterruptPending = true; - LogMemoryContextPending = true; - /* latch will be set by procsignal_sigusr1_handler */ -} - /* * ProcessLogMemoryContextInterrupt * Perform logging of memory contexts of this backend process. @@ -1040,8 +1024,6 @@ HandleLogMemoryContextInterrupt(void) void ProcessLogMemoryContextInterrupt(void) { - LogMemoryContextPending = false; - ereport(LOG, (errmsg("logging memory contexts of PID %d", MyProcPid))); diff --git a/src/include/access/parallel.h b/src/include/access/parallel.h index 93d88ac60e..1239d45cb5 100644 --- a/src/include/access/parallel.h +++ b/src/include/access/parallel.h @@ -71,7 +71,6 @@ extern void WaitForParallelWorkersToFinish(ParallelContext *pcxt); extern void DestroyParallelContext(ParallelContext *pcxt); extern bool ParallelContextActive(void); -extern void HandleParallelMessageInterrupt(void); extern void HandleParallelMessages(void); extern void AtEOXact_Parallel(bool isCommit); extern void AtEOSubXact_Parallel(bool isCommit, SubTransactionId mySubId); diff --git a/src/include/commands/async.h b/src/include/commands/async.h index f371ac896b..76532aaaeb 100644 --- a/src/include/commands/async.h +++ b/src/include/commands/async.h @@ -21,7 +21,6 @@ #define NUM_NOTIFY_BUFFERS 8 extern bool Trace_notify; -extern volatile sig_atomic_t notifyInterruptPending; extern Size AsyncShmemSize(void); extern void AsyncShmemInit(void); @@ -44,9 +43,6 @@ extern void AtSubCommit_Notify(void); extern void AtSubAbort_Notify(void); extern void AtPrepare_Notify(void); -/* signal handler for inbound notifies (PROCSIG_NOTIFY_INTERRUPT) */ -extern void HandleNotifyInterrupt(void); - /* process interrupts */ extern void ProcessNotifyInterrupt(bool flush); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 90a3016065..002d515fb0 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -27,6 +27,7 @@ #include "datatype/timestamp.h" /* for TimestampTz */ #include "pgtime.h" /* for pg_time_t */ +#include "storage/procsignal.h" #define InvalidPid (-1) @@ -85,19 +86,6 @@ * *****************************************************************************/ -/* in globals.c */ -/* these are marked volatile because they are set by signal handlers: */ -extern PGDLLIMPORT volatile sig_atomic_t InterruptPending; -extern PGDLLIMPORT volatile sig_atomic_t QueryCancelPending; -extern PGDLLIMPORT volatile sig_atomic_t ProcDiePending; -extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending; -extern PGDLLIMPORT volatile sig_atomic_t IdleSessionTimeoutPending; -extern PGDLLIMPORT volatile sig_atomic_t ProcSignalBarrierPending; -extern PGDLLIMPORT volatile sig_atomic_t LogMemoryContextPending; - -extern PGDLLIMPORT volatile sig_atomic_t CheckClientConnectionPending; -extern PGDLLIMPORT volatile sig_atomic_t ClientConnectionLost; - /* these are marked volatile because they are examined by signal handlers: */ extern PGDLLIMPORT volatile uint32 InterruptHoldoffCount; extern PGDLLIMPORT volatile uint32 QueryCancelHoldoffCount; @@ -107,14 +95,8 @@ extern PGDLLIMPORT volatile uint32 CritSectionCount; extern void ProcessInterrupts(void); /* Test whether an interrupt is pending */ -#ifndef WIN32 #define INTERRUPTS_PENDING_CONDITION() \ - (unlikely(InterruptPending)) -#else -#define INTERRUPTS_PENDING_CONDITION() \ - (unlikely(UNBLOCKED_SIGNAL_QUEUE()) ? pgwin32_dispatch_queued_signals() : 0, \ - unlikely(InterruptPending)) -#endif + (unlikely(ProcSignalAnyPending())) /* Service interrupt, if one is pending and it's safe to service it now */ #define CHECK_FOR_INTERRUPTS() \ diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h index 6d4122b4c7..52dddfacd9 100644 --- a/src/include/postmaster/bgworker.h +++ b/src/include/postmaster/bgworker.h @@ -98,6 +98,7 @@ typedef struct BackgroundWorker Datum bgw_main_arg; char bgw_extra[BGW_EXTRALEN]; pid_t bgw_notify_pid; /* SIGUSR1 this backend on start/stop */ + int bgw_notify_pgprocno; /* set this proc's latch on start/stop */ } BackgroundWorker; typedef enum BgwHandleStatus diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h index eec186be2e..4244333436 100644 --- a/src/include/storage/procsignal.h +++ b/src/include/storage/procsignal.h @@ -36,6 +36,14 @@ typedef enum PROCSIG_BARRIER, /* global barrier interrupt */ PROCSIG_LOG_MEMORY_CONTEXT, /* ask backend to log the memory contexts */ + PROCSIG_IDLE_SESSION_TIMEOUT, + PROCSIG_IDLE_TRANSACTION_TIMEOUT, + PROCSIG_CHECK_CONNECTION_TIMEOUT, + PROCSIG_CONNECTION_LOST, + PROCSIG_QUERY_CANCEL, + PROCSIG_DIE, + PROCSIG_RECOVERY_CONFLICT, + /* Recovery conflict reasons */ PROCSIG_RECOVERY_CONFLICT_DATABASE, PROCSIG_RECOVERY_CONFLICT_TABLESPACE, @@ -71,6 +79,11 @@ extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type); extern void WaitForProcSignalBarrier(uint64 generation); extern void ProcessProcSignalBarrier(void); -extern void procsignal_sigusr1_handler(SIGNAL_ARGS); +extern bool ProcSignalAnyPending(void); +extern void ProcSignalClearAnyPending(void); +extern bool ProcSignalPending(ProcSignalReason reason); +extern bool ProcSignalConsume(ProcSignalReason reason); +extern void ProcSignalClear(ProcSignalReason reason); +extern void ProcSignalRaise(ProcSignalReason reason); #endif /* PROCSIGNAL_H */ diff --git a/src/include/storage/sinval.h b/src/include/storage/sinval.h index f03dc23b14..d476543b52 100644 --- a/src/include/storage/sinval.h +++ b/src/include/storage/sinval.h @@ -125,16 +125,11 @@ 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); extern void ReceiveSharedInvalidMessages(void (*invalFunction) (SharedInvalidationMessage *msg), void (*resetFunction) (void)); -/* signal handler for catchup events (PROCSIG_CATCHUP_INTERRUPT) */ -extern void HandleCatchupInterrupt(void); - /* * enable/disable processing of catchup events directly from signal handler. * The enable routine first performs processing of any catchup events that -- 2.30.2