public inbox for [email protected]
help / color / mirror / Atom feedFrom: Denis Laxalde <[email protected]>
To: Jelte Fennema <[email protected]>
Cc: Greg Stark <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Gregory Stark (as CFM) <[email protected]>
Cc: Jelte Fennema <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
Date: Tue, 28 Mar 2023 16:53:18 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAGECzQTaGtAWmM98q9uup2pjODR8WSOsF_UV3YG7THoqatJApA@mail.gmail.com>
References: <CAM-w4HP4yN2krwqhXUBQYCjzSjCAWyXMb1xAOo4cv_-_hLXUWA@mail.gmail.com>
<CAGECzQRZTyPVGLANavQ7uokDywx-bodpK=NSWqgxM-mnJ_foQg@mail.gmail.com>
<CAM-w4HNCHEuCBcUxE8H6t15SJKzZd6hKtyHNUTa30VhJxqmuOw@mail.gmail.com>
<CAGECzQSvTjte1-QmzcjnJJkr7BYvuOK4FkjXpXVnRA2AJHuAHw@mail.gmail.com>
<CAGECzQR57o1wdo9fG7grmmabrt1jotHR-T=nQZGXiZRHpvgrGg@mail.gmail.com>
<CAM-w4HOf4+em6TivqZXtbVvpahHRYExxosK5eAyje5C+dSxWSg@mail.gmail.com>
<[email protected]>
<CAM-w4HPyCqVuf=fR-VBB1xcCtUF0+cefpVOv29O-JYbAG4dh2w@mail.gmail.com>
<CAGECzQR7kRY3WKKwx0FdmAA3P2vD+SGALOcXV-dcqMwM4LHV8A@mail.gmail.com>
<CAGECzQTaGtAWmM98q9uup2pjODR8WSOsF_UV3YG7THoqatJApA@mail.gmail.com>
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
Attachments:
[text/x-diff] 0001-fixup-Add-non-blocking-version-of-PQcancel.patch (3.3K, ../[email protected]/2-0001-fixup-Add-non-blocking-version-of-PQcancel.patch)
download | inline diff:
From a5f9cc680ffa520b05fe34b7cac5df2e60a6d4ad Mon Sep 17 00:00:00 2001
From: Denis Laxalde <[email protected]>
Date: Tue, 28 Mar 2023 16:06:42 +0200
Subject: [PATCH] fixup! Add non-blocking version of PQcancel
---
doc/src/sgml/libpq.sgml | 16 +++++++++++++++-
src/interfaces/libpq/fe-connect.c | 2 +-
src/test/modules/libpq_pipeline/libpq_pipeline.c | 2 +-
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 29f08a4317..aa404c4d15 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5795,7 +5795,7 @@ PGcancelConn *PQcancelSend(PGconn *conn);
options as the the original <structname>PGconn</structname>. So when the
original connection is encrypted (using TLS or GSS), the connection for
the cancel request is encrypted in the same way. Any connection options
- that only are only used during authentication or after authentication of
+ that are only used during authentication or after authentication of
the client are ignored though, because cancellation requests do not
require authentication and the connection is closed right after the
cancellation request is submitted.
@@ -5912,6 +5912,20 @@ ConnStatusType PQcancelStatus(const PGcancelConn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQcancelSocket">
+ <term><function>PQcancelSocket</function><indexterm><primary>PQcancelSocket</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ A version of <xref linkend="libpq-PQsocket"/> that can be used for
+ cancellation connections.
+<synopsis>
+int PQcancelSocket(PGcancelConn *conn);
+</synopsis>
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQcancelPoll">
<term><function>PQcancelPoll</function><indexterm><primary>PQcancelPoll</primary></indexterm></term>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 74e337fddf..16af7303d4 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -808,7 +808,7 @@ PQcancelConn(PGconn *conn)
/*
* Cancel requests should not iterate over all possible hosts. The request
* needs to be sent to the exact host and address that the original
- * connection used. So we we manually create the host and address arrays
+ * connection used. So we manually create the host and address arrays
* with a single element after freeing the host array that we generated
* from the connection options.
*/
diff --git a/src/test/modules/libpq_pipeline/libpq_pipeline.c b/src/test/modules/libpq_pipeline/libpq_pipeline.c
index e8e904892c..6764ab513b 100644
--- a/src/test/modules/libpq_pipeline/libpq_pipeline.c
+++ b/src/test/modules/libpq_pipeline/libpq_pipeline.c
@@ -89,7 +89,7 @@ pg_fatal_impl(int line, const char *fmt,...)
/*
* Check that the query on the given connection got cancelled.
*
- * This is a function wrapped in a macrco to make the reported line number
+ * This is a function wrapped in a macro to make the reported line number
* in an error match the line number of the invocation.
*/
#define confirm_query_cancelled(conn) confirm_query_cancelled_impl(__LINE__, conn)
--
2.30.2
view thread (12+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox