Re: Add non-blocking version of PQcancel - Mailing list pgsql-hackers

From Jelte Fennema
Subject Re: Add non-blocking version of PQcancel
Date
Msg-id HE1PR8303MB00735DC38F45BD63A3A49347F71F9@HE1PR8303MB0073.EURPRD83.prod.outlook.com
Whole thread Raw
In response to Re: [EXTERNAL] Re: Add non-blocking version of PQcancel  (Jelte Fennema <Jelte.Fennema@microsoft.com>)
Responses Re: Add non-blocking version of PQcancel
List pgsql-hackers
I attached a new version of this patch. Which does three main things:
1. Change the PQrequestCancel implementation to use the regular
    connection establishement code, to support all connection options
    including encryption.
2. Add PQrequestCancelStart which is a thread-safe and non-blocking
    version of this new PQrequestCancel implementation.
3. Add PQconnectComplete, which completes a connection started by
    PQrequestCancelStart. This is useful if you want a thread-safe, but
    blocking cancel (without having a need for signal safety).

This change un-deprecates PQrequestCancel, since now there's actually an
advantage to using it over PQcancel. It also includes user facing documentation
for all these functions.

As a API design change from the previous version, PQrequestCancelStart now
returns a regular PGconn for the cancel connection.

@Tom Lane regarding this:
> Even in the non-blocking case, callers should only deal with the original PGconn.

This would by definition result in non-threadsafe code (afaict). So I refrained from doing this.
The blocking version doesn't expose a PGconn at all, but the non-blocking one now returns a new PGconn.

There's two more changes that I at least want to do before considering this patch mergable:
1. Go over all the functions that can be called with a PGconn, but should not be
    called with a cancellation PGconn and error out or exit early.
2. Copy over the SockAddr from the original connection and always connect to
    the same socket. I believe with the current code the cancellation could end up
    at the wrong server if there are multiple hosts listed in the connection string.

And there's a third item that I would like to do as a bonus:
3. Actually use the non-blocking API for the postgres_fdw code to implement a
    timeout. Which would allow this comment can be removed:
    /*
     * Issue cancel request.  Unfortunately, there's no good way to limit the
     * amount of time that we might block inside PQgetCancel().
     */

So a next version of this patch can be expected somewhere later this week.
But any feedback on the current version would be appreciated. Because
these 3 changes won't change the overall design much.

Jelte
Attachment

pgsql-hackers by date:

Previous
From: Justin Pryzby
Date:
Subject: Re: Printing backtrace of postgres processes
Next
From: "David G. Johnston"
Date:
Subject: Re: Returning multiple rows in materialized mode inside the extension