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
- 0001-Remove-unused-latch.patch (text/x-patch) patch 0001
- 0002-Remove-unneeded-include.patch (text/x-patch) patch 0002
- 0003-Rename-SetWalSummarizerLatch-function.patch (text/x-patch) patch 0003
- 0004-Address-walwriter-and-checkpointer-by-proc-number.patch (text/x-patch) patch 0004
- 0005-Address-walreceiver-by-procno-not-direct-pointer-to-.patch (text/x-patch) patch 0005
- 0006-Use-proc-number-for-wakeups-in-waitlsn.c.patch (text/x-patch) patch 0006
- 0007-Replace-Latches-with-Interrupts.patch (text/x-patch) patch 0007
- 0008-Replace-ProcSendSignal-ProcWaitForSignal-with-new-in.patch (text/x-patch) patch 0008
- 0009-Replace-all-remaining-Latch-calls-with-new-Interrupt.patch (text/x-patch) patch 0009
- 0010-Fix-lost-wakeup-issue-in-logical-replication-launche.patch (text/x-patch) patch 0010
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
-
Ignore SIGINT in walwriter and walsummarizer
- a92b809f9da1 19 (unreleased) landed
-
Split WaitEventSet functions to separate source file
- 393e0d231405 18.0 landed
-
Use ModifyWaitEvent to update exit_on_postmaster_death
- 84e5b2f07a5e 18.0 landed
-
Remove unused ShutdownLatchSupport() function
- a98e4dee63ce 18.0 landed
-
Rename two functions that wake up other processes
- 368d8270c838 18.0 landed
-
Use ProcNumbers instead of direct Latch pointers to address other procs
- a9c546a5a378 18.0 landed
-
Clean up WaitLatch calls that passed latch without WL_LATCH_SET
- f9ecb57a506a 18.0 landed
-
Remove unneeded #include
- 094ae071605d 18.0 landed
-
Remove unused latch
- 6c0c49f7d37d 18.0 landed
-
Remove support for background workers without BGWORKER_SHMEM_ACCESS.
- 80a8f95b3bca 15.0 cited