Re: Converting pqsignal to void return

> On Jan 22, 2025, at 9:36 AM, Andres Freund <andres@anarazel.de> wrote:
> 
> Hi,
> 
> On 2025-01-22 10:22:45 -0600, Nathan Bossart wrote:
>> On Wed, Jan 22, 2025 at 07:57:52AM -0800, Paul Ramsey wrote:
>>> The problem we are having in extension land is that we often run
>>> functions in external libraries that take a long time to return, and we
>>> would like to ensure that PgSQL users can cancel their queries, even when
>>> control has passed into those functions.
>>> 
>>> The way we have done it, historically, has been to take the return value
>>> of pqsignal(SIGINT, extension_signint_handler) and remember it, and then,
>>> inside extension_signint_handler, call the pgsql handler once we have
>>> done our own business..
>> 
>> I see a couple other projects that might be doing something similar [0].
>> 
>> [0] https://codesearch.debian.net/search?q=%3D+pqsignal&literal=1
> 
> Grouping by package I see three packages.

So many things for me to atone for! :)

> All these seem to not handle backend termination, which certainly doesn't seem
> optimal - it e.g. means that a fast shutdown won't really work.
> 
> 1) postgis
> 
>   For at least some of the cases it doesn't look trivial to convert to
>   checking QueryCancelPending/ProcDiePending because the signal handler calls
>   into GEOS and lwgeom.

We control both those libraries, so we just may need to change them up a little to maybe pass in an callback for their internal check-for-interrupt to call.

> 2) psql-http
> 
>   Also doesn't handle termination.
>   Looks like it could trivially be converted to checking
>   QueryCancelPending/ProcDiePending.

Going to fix this one first.

> 3) timescaledb

Not me!

> IOW, the only worrisome case here is postgis.
> Given that we are working towards *not* relying on signals for a lot of this,
> I wonder if we ought to support registering callbacks to be called on receipt
> of query cancellation and backend termination. That then could e.g. call
> GEOS_interruptRequest() as postgis does.  That'd have a chance of working in a
> threaded postgres too - unfortunately it looks like neither lwgeom's nor
> GEOS's interrupt mechanisms are thread safe at this point.

I think callbacks are a good thing. Two of the libraries of interest to me (curl, GDAL) I end up using the progress meter callback to wedge myself in and force an interrupt as necessary. This seems like a common feature of libraries with long running work. 

ATB,

P

> 
> 
> It's worth noting that postgis ends up with a bunch of postgres-internals
> knowledge due to its desire to handle signals:
> https://github.com/postgis/postgis/blob/master/postgis/postgis_module.c#L51-L56
> 
> #ifdef WIN32
> static void interruptCallback() {
>  if (UNBLOCKED_SIGNAL_QUEUE())
>    pgwin32_dispatch_queued_signals();
> }
> #endif
> 
> Which seems really rather undesirable.
> 
> 
> But given how windows signals are currently delivered via the equivalent code
> in CHECK_FOR_INTERRUPTS(), I don't really see an alternative at this point :(.
> 
> 
> 
> 
> Greetings,
> 
> Andres Freund