From: | Denis Laxalde <denis(dot)laxalde(at)dalibo(dot)com> |
---|---|
To: | Jelte Fennema <postgres(at)jeltef(dot)nl> |
Cc: | Greg Stark <stark(at)mit(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Gregory Stark (as CFM)" <stark(dot)cfm(at)gmail(dot)com>, Jelte Fennema <Jelte(dot)Fennema(at)microsoft(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Jacob Champion <jchampion(at)timescale(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [EXTERNAL] Re: Add non-blocking version of PQcancel |
Date: | 2023-03-28 14:53:18 |
Message-ID: | 20230328145318.upvrlslcbjyh47ce@dalibo.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Jelte,
I had a look into your patchset (v16), did a quick review and played a
bit with the feature.
Patch 2 is missing the documentation about PQcancelSocket() and contains
a few typos; please find attached a (fixup) patch to correct these.
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -321,16 +328,28 @@ extern PostgresPollingStatusType PQresetPoll(PGconn *conn);
/* Synchronous (blocking) */
extern void PQreset(PGconn *conn);
+/* issue a cancel request */
+extern PGcancelConn * PQcancelSend(PGconn *conn);
[...]
Maybe I'm missing something, but this function above seems a bit
strange. Namely, I wonder why it returns a PGcancelConn and what's the
point of requiring the user to call PQcancelStatus() to see if something
got wrong. Maybe it could be defined as:
int PQcancelSend(PGcancelConn *cancelConn);
where the return value would be status? And the user would only need to
call PQcancelErrorMessage() in case of error. This would leave only one
single way to create a PGcancelConn value (i.e. PQcancelConn()), which
seems less confusing to me.
Jelte Fennema wrote:
> Especially since I ran into another use case that I would want to use
> this patch for recently: Adding an async cancel function to Python
> it's psycopg3 library. This library exposes both a Connection class
> and an AsyncConnection class (using python its asyncio feature). But
> one downside of the AsyncConnection type is that it doesn't have a
> cancel method.
As part of my testing, I've implemented non-blocking cancellation in
Psycopg, based on v16 on this patchset. Overall this worked fine and
seems useful; if you want to try it:
https://github.com/dlax/psycopg3/tree/pg16/non-blocking-pqcancel
(The only thing I found slightly inconvenient is the need to convey the
connection encoding (from PGconn) when handling error message from the
PGcancelConn.)
Cheers,
Denis
Attachment | Content-Type | Size |
---|---|---|
0001-fixup-Add-non-blocking-version-of-PQcancel.patch | text/x-diff | 3.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jehan-Guillaume de Rorthais | 2023-03-28 14:56:17 | Re: Memory leak from ExecutorState context? |
Previous Message | Jonathan S. Katz | 2023-03-28 14:51:50 | Re: Support logical replication of DDLs |