From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Paul Ramsey <pramsey(at)cleverelephant(dot)ca> |
Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Subject: | Re: Converting pqsignal to void return |
Date: | 2025-01-22 18:15:59 |
Message-ID: | o2l2hjme3elgidkyap6xgd7huznzjfgunutdznio7upngsf7xx@t6jjbfkirfmf |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2025-01-22 09:50:18 -0800, Paul Ramsey wrote:
> > On Jan 22, 2025, at 9:36 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > 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! :)
Turns out creating good stuff almost invariably leeds to creating not so great
parts of those good things :)
> > 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.
Yea, makes sense. Looking at the callsites for the interrupt checking, it
doesn't look like a function call during interrupt checking will have a
meaningful performance impact.
> > 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.
Makes sense.
> > 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.
As I mentioned earlier, for something like curl, where afaict you'd want to
accept interrupts while waiting for network IO, I think it's better to convert
to non-blocking use of such APIs and checking interrupt in the event
loop. It's been a while, but I have done that with curl, and it wasn't
particularly hard.
That approach imo is already cleaner today and I think it's what we'll
eventually require for at least a bunch of architectural improvements:
- faster query execution (so that execution can switch to another part of the
query tree while blocked on network IO)
- connection scalability (if there's no 1:1 mapping between sessions and
processes/threads, threads have to switch between executing queries for
different sessions)
- to benefit from asynchronous completion-based FE/BE network handling.
But of course all of these are all quite a bit away.
Obviously event based loops are not a viable approach for wanting to check for
interrupts in, what I assume to be CPU bound, cases like GDAL/lwgeom.
If you have a "progress meter callback", I don't think there's really a
benefit in postgres providing a way to register a callback when receiving a
cancellation / termination event?
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2025-01-22 18:16:03 | Re: pg_attribute_noreturn(), MSVC, C11 |
Previous Message | Peter Eisentraut | 2025-01-22 18:08:10 | Re: Update Unicode data to Unicode 16.0.0 |