Re: Interrupts vs signals
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Robert Haas
<robertmhaas@gmail.com>, Fujii Masao <masao.fujii@gmail.com>,
pgsql-hackers <pgsql-hackers@postgresql.org>,
Andres Freund <andres@anarazel.de>
Date: 2024-12-02T14:39:28Z
Lists: pgsql-hackers
Attachments
- v5-0001-Replace-Latches-with-Interrupts.patch (text/x-patch) patch v5-0001
- v5-0002-Fix-lost-wakeup-issue-in-logical-replication-laun.patch (text/x-patch) patch v5-0002
- v5-0003-Use-INTERRUPT_GENERAL-for-bgworker-state-change-n.patch (text/x-patch) patch v5-0003
On 02/12/2024 12:42, Heikki Linnakangas wrote: > On 02/12/2024 09:32, Thomas Munro wrote: >> On Sat, Nov 23, 2024 at 10:58 AM Heikki Linnakangas <hlinnaka@iki.fi> >> wrote: >>> Hmm, so this would replace the maybeSleepingOnInterrupts bitmask I >>> envisioned. Makes a lot of sense. If it's a single bit though, that >>> means that you'll still get woken up by interrupts that you're not >>> waiting for. Maybe that's fine. Or we could merge the >>> maybeSleepingOnInterrupts and pendingInterrupts bitmasks to a single >>> atomic word, so that you would have a separate "maybe sleeping" bit for >>> each interrupt bit, but could still use atomic_fetch_or atomically read >>> the interrupt bits and announce the sleeping. >> >> I think one bit is fine for now. At least, until we have a serious >> problem with interrupts arriving when you're sleeping but not ready to >> service that particular interrupt. The 'interrupt bit already set, >> don't try to wake me' stuff discussed earlier would limit the number >> of useless wakeups to one, until you eventually are ready and consume >> the interrupt. The main case I can think of, if we fast forward to >> the all-procsignals-become-interrupts patch (which I'll be rebasing on >> top of this when the next version appears), is that you might receive >> a sinval catchup request, but you might be busy running a long query. >> Sinval catchup messages are only processed between queries, so you >> just keep ignoring them until end of query. I think that's fine, and >> unlikely. Do you have other cases in mind? > > Yeah, no, I think one bit is is good enough. Let's go with that. Here's a new patch set version, with the following changes: - Implement the "maybe sleeping" flag as a single bit in the pending interrupts mask, per above discussion. One notable change is that I moved the check for whether an interrupt is set out of the loop in WaitEventSetWait(). It seemed redundant; all the WaitEventSetWaitBlock() implementations also check the interrupt mask if the wakeup is received. I'm sure it doesn't make a difference from performance point of view, but it feels more natural to me this way. - Suppress the wakeup in SendInterrupt if the interrupt was already pending, per discussion - Rename INTERRUPT_GENERAL_WAKEUP to INTERRUPT_GENERAL per Robert's suggestion - Fix a bunch of minor comment issues, some pointed out off-list by Álvaro (thanks!) -- 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