Re: Interrupts vs signals

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Interrupts vs signals
Date: 2024-11-04 19:41:33
Message-ID: b697b96e-78b5-49bb-8d66-581143a65d7a@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)iki(dot)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)

Attachment Content-Type Size
v3-0001-Replace-Latches-with-Interrupts.patch text/x-patch 223.2 KB
v3-0002-Fix-lost-wakeup-issue-in-logical-replication-laun.patch text/x-patch 4.0 KB
adapt-pg_cron.patch text/x-patch 3.5 KB
pg_version_compatibility.h text/x-chdr 3.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-11-04 19:51:10 Re: relfilenode statistics
Previous Message Tom Lane 2024-11-04 19:18:17 Re: Allow specifying a dbname in pg_basebackup connection string