Re: Interrupts vs signals
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>
Cc: 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-11-04T19:41:33Z
Lists: pgsql-hackers
Attachments
- v3-0001-Replace-Latches-with-Interrupts.patch (text/x-patch) patch v3-0001
- v3-0002-Fix-lost-wakeup-issue-in-logical-replication-laun.patch (text/x-patch) patch v3-0002
- adapt-pg_cron.patch (text/x-patch) patch
- pg_version_compatibility.h (text/x-chdr)
On 31/10/2024 02:32, Michael Paquier wrote: > 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: >>> 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. Having spent some time playing with this, I quite like option C: break compatibility, but provide an out-of-tree header file with *forward*-compatibility macros. That encourages extension authors to adapt to new idioms, but avoids having to sprinkle extension code with #if version checks to support old versions. See attached pg_version_compatibility.h header. It allows compiling code that uses basic SendInterrupt, RaiseInterrupt, WaitInterrupt calls with older server versions. My plan is to put this on the Wiki, and update it with similar compatibility helpers for other changes we might make in v18 or future versions. We could add helpers for previous incompatibilities in v17 and v16 too, although at quick glance I'm not sure what those might be. I tested this approach by adapting pg_cron to build with these patches and the compatibility header, and compiling it with all supported server versoins. Works great, see adapt-pg_cron.patch. I pushed the preliminary cleanup patches from this patch set earlier, only the main patches remain. Attached is a new version of those, with mostly comment cleanups. -- 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