Re: Interrupts vs signals

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-08-24T17:17:47Z
Lists: pgsql-hackers

Attachments

On 07/08/2024 17:59, Heikki Linnakangas wrote:
> I'm also wondering about the relationship between interrupts and 
> latches. Currently, SendInterrupt sets a latch to wake up the target 
> process. I wonder if it should be the other way 'round? Move all the 
> wakeup code, with the signalfd, the self-pipe etc to interrupt.c, and in 
> SetLatch, call SendInterrupt to wake up the target process? Somehow that 
> feels more natural to me, I think.

I explored that a little, see attached patch set. It's going towards the 
same end state as your patches, I think, but it starts from different 
angle. In a nutshell:

Remove Latch as an abstraction, and replace all use of Latches with 
Interrupts. When I originally created the Latch abstraction, I imagined 
that we would have different latches for different purposes, but in 
reality, almost all code just used the general-purpose "process latch". 
this patch accepts that reality and replaces the Latch struct directly 
with the interrupt mask in PGPROC.

This initially defines only two interrupts. INTERRUPT_GENERAL_WAKEUP is 
the main one, sending that interrupt to a process replaces setting the 
process's generic process latch in PGPROC:

* SetLatch(MyLatch) -> RaiseInterrupt(INTERRUPT_GENERAL_WAKEUP)

* SetLatch(&ProcGlobal->allProcs[procno].procLatch) -> 
SendInterrupt(procno, INTERRUPT_GENERAL_WAKEUP

* ResetLatch(MyLatch) -> ClearInterrupt(INTERRUPT_GENERAL_WAKEUP)

* WaitLatch(MyLatch) -> WaitInterrupt(1 << INTERRUPT_GENERAL_WAKEUP)

There was only one extra Latch in addition the process's generic 
procLatch, the recoveryWakeupLatch. It's replaced by the second 
interrupt bit, INTERRUPT_RECOVERY_WAKEUP.

This is complementary or preliminary work to your patch set. All the 
changes to replace ProcSignals with different interrupt bits could go on 
top of this.

This patch set is work in progress, I'd love to hear your thoughts on 
this before I spent more time on this. (Haven't tested on Windows for 
example).

Patches 0001 - 0006 are just little cleanups and minor refactorings that 
I think make sense even without the rest of the work, though.

0007 is the main patch.

Patch 0010 addresses the problem discussed at 
https://www.postgresql.org/message-id/CALDaNm01_KEgHM1tKtgXkCGLJ5209SMSmGw3UmhZbOz365_%3DeA%40mail.gmail.com. 
Other solutions are discussed on that thread, but while working on 
these, I realized that with these new interrupts, it's pretty 
straightforward to fix by introducing one more interrupt reason.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

Commits

  1. Ignore SIGINT in walwriter and walsummarizer

  2. Split WaitEventSet functions to separate source file

  3. Use ModifyWaitEvent to update exit_on_postmaster_death

  4. Remove unused ShutdownLatchSupport() function

  5. Rename two functions that wake up other processes

  6. Use ProcNumbers instead of direct Latch pointers to address other procs

  7. Clean up WaitLatch calls that passed latch without WL_LATCH_SET

  8. Remove unneeded #include

  9. Remove unused latch

  10. Remove support for background workers without BGWORKER_SHMEM_ACCESS.