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

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

  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.