Re: Interrupts vs signals

On Mon, 4 Nov 2024 at 20:42, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> 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.

+1 maintaining a subset of these things for every extension is kind of a pain

> My plan is to put this on the Wiki

Why the wiki and not as a file in the repo? Seems like it would be
nice to update this file together with patches that introduce such
breakages. To be clear, I think it shouldn't be possible to #include
the file, such a forward compatibility file should always be
copy-pasted. But having it in the same place as the code seems useful,
just like we update docs together with the code.

> We could add helpers for previous
> incompatibilities in v17 and v16 too, although at quick glance I'm not
> sure what those might be.

One thing that I personally add to any extension I maintain is the new
foreach macros introduced in PG17, because they are just so much nicer
to use.

> 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.

Looks like a simple change indeed.

On Mon, 4 Nov 2024 at 20:42, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
> 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)