From: | Jelte Fennema <Jelte(dot)Fennema(at)microsoft(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Jacob Champion <pchampion(at)vmware(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Add non-blocking version of PQcancel |
Date: | 2022-03-30 16:08:16 |
Message-ID: | HE1PR8303MB00735DC38F45BD63A3A49347F71F9@HE1PR8303MB0073.EURPRD83.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 | Content-Type | Size |
---|---|---|
0001-Add-documentation-for-libpq_pipeline-tests.patch | application/octet-stream | 1.5 KB |
0002-Add-non-blocking-version-of-PQcancel.patch | application/octet-stream | 39.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2022-03-30 16:12:54 | Re: Returning multiple rows in materialized mode inside the extension |
Previous Message | Justin Pryzby | 2022-03-30 16:03:08 | Re: Printing backtrace of postgres processes |