Re: [PATCH] Optimize ProcSignal to avoid redundant SIGUSR1 signals - Mailing list pgsql-hackers

On Wed, Jul 23, 2025 at 8:08 AM Joel Jacobson <joel@compiler.org> wrote:
> Previously, ProcSignal used an array of volatile sig_atomic_t flags, one
> per signal reason. A sender would set a flag and then unconditionally
> send a SIGUSR1 to the target process. This could result in a storm of
> redundant signals if multiple processes signaled the same target before
> it had a chance to run its signal handler.

It's a good idea with great results!

FWIW the "new interrupts systems" patch set[1] also contains this
idea.  But it goes further, because it also removes all the
corresponding "pending" local variables: notice how
HandleNotifyInterrupt() really just sets notifyInterruptPending = true
and does SetLatch(MyLatch).  So instead of having
CHECK_FOR_INTERRUPTS() check all those local variables, we can merge
"procsignals" with "interrupts", and have CHECK_FOR_INTERRUPTS()
itself read that atomic word directly, skipping the middleman and
deleting the whole ProcSignal system.  Originally I proposed that the
sending backend would now do SendInterrupt() directly, essentially
moving the contents of the signal handler into the sender and setting
the receiver's latch directly.  But then Heikki went further and
proposed deleting latches too, and having "interrupts" as our only
remaining inter-backend wakeup abstraction, with one of the interrupt
flags used as a "general" one that replaces all the places that use
SetLatch() directly today.

The end state could include a WaitEventSetWait() implementation that
waits for that single remaining atomic word with a (multiplexed) futex
wait, also removing the "maybe sleeping" dance because futexes already
have ideal protection against races.  Or you can send a byte to a pipe
or post a custom event to an io_uring or kqueue or whatever.

That's part of a larger project to get rid of all inter-backend
signals, on the pathway to multi-threaded backends.  Some things along
the way have included making it true that all real
IPC-request-handling work is done in CHECK_FOR_INTERRUPTS(), not
signal handlers (ie making it all "cooperative"), and making it safe
to use uint32_t atomics at all in signal handlers as you're doing here
(previously they were allowed to be emulated with spinlocks), as an
intermediate step because in the short term we still have signals for
timers.

I'm planning to post a rebase of that work with some improvements
soon.  Hmm, I wonder if there is any sense in restructuring it so that
your patch becomes an incremental stepping stone.  I think one of our
problems might have been trying to change too many things at once.
What you're doing is certainly independently good, but I haven't
studied your patch too closely yet.

[1] https://www.postgresql.org/message-id/flat/CA%2BhUKG%2B3MkS21yK4jL4cgZywdnnGKiBg0jatoV6kzaniBmcqbQ%40mail.gmail.com