Re: Interrupts vs signals

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Thomas Munro <thomas.munro@gmail.com>, Fujii Masao <masao.fujii@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-10-31T00:32:16Z
Lists: pgsql-hackers
On Wed, Oct 30, 2024 at 01:23:54PM -0400, Robert Haas wrote:
> On Wed, Oct 30, 2024 at 12:03 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> We can provide backwards compatibility macros and a new facility to
>> allocate custom interrupt bits. But how big of a problem is this anyway?
>> In an in-person chat last week, Andres said something like "this will
>> break every extension".
> 
> Seems hyperbolic.

Most extensions that rely on bgworkers, at least.

> However, that was also pretty easy to fix. This seems like it might be
> a little more complicated.

The advantage of the breakage is also to force extension maintainers
to look at the changes because these have benefits.  I'd like to think
that 6f3bd98ebfc0 was a good move overall even if it broke come
compilations and these require more PG_VERSION_NUM magic.  For the
extension maintainer hat on, it is always annoying, but not really
that bad in the long-term.

>> We have a few options for how to deal with backwards-compatibility for
>> extensions:
>>
>> A) If we rip off the bandaid in one go and don't provide any
>> backwards-compatibility macros, we will break 96 extensions. Most of
>> them can be fixed by replacing WaitLatch, SetLatch, ResetLatch with
>> corresponding WaitInterrupt, RaiseInterrupt, ResetInterrupt calls. (With
>> #ifdef for compatibility with older postgres versions)
>>
>> B) If we provide backwards-compatibility macros so that simple Latch
>> calls on MyLatch continue working, we will break about 14 extensions.
>> They will need some tweaking like in option A). A bit more than the
>> simple cases in option A), but nothing too difficult.
>>
>> C) We could provide "forward-compatibility" macros in a separate header
>> file, to make the new "SetInterrupt" etc calls work in old PostgreSQL
>> versions. Many of the extensions already have a header file like this,
>> see e.g. citusdata/citus/src/include/pg_version_compat.h,
>> pipelinedb/pipelinedb/include/compat.h. It might actually be a good idea
>> to provide a semi-official header file like this on the Postgres wiki,
>> to help extension authors. It would encourage extensions to use the
>> latest idioms, while still being able to compile for older versions.
>>
>> I'm leaning towards option C). Let's rip off the band-aid, but provide
>> documentation for how to adapt your extension code. And provide a
>> forwards-compatibility header on the wiki, that extension authors can
>> use to make the new Interrupt calls work against old server versions.
> 
> I don't know which of these options is best, but I don't find any of
> them categorically unacceptable.

Looking at the compatibility macros of 0008 for the latches with
INTERRUPT_GENERAL_WAKEUP under latch.h, the changes are not that bad
to adapt to, IMO.  It reminds of f25968c49697: hard breakage, no
complaints I've heard of because I guess that most folks have been
using an in-house compatibility headers.

A big disadvantage of B is that someone may decide to add new code in
core that depends on the past routines, and we'd better avoid that for
this new layer of APIs for interrupt handling.  A is a subset of C: do
a hard switch in the core code, with C mentioning a compatibility
layer in the wiki that does not exist in the core code.  Any of A or C
is OK, I would not choose B for the core backend.
--
Michael

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.