From: Tristan Partin <tristan@neon.tech>
To: Robert Haas <robertmhaas@gmail.com>
To: Jelte Fennema-Nio <postgres@jeltef.nl>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Cc: Shlok Kyal <shlok.kyal.oss@gmail.com>
Subject: Re: psql not responding to SIGINT upon db reconnection
Date: Fri, 22 Mar 2024 12:05:20 -0500
Message-ID: <D00FXVDVQG85.1CBNG8IZ2XM8E@neon.tech> (raw)
In-Reply-To: <CA+Tgmoa1Uaqnd_OnV=KrQ5qf3=wYv8JM=3SC1e_=0xo0o9KEjg@mail.gmail.com>
References: <CUAJJDVD8L61.2AH6VEEDMTXX3@gonk>
<CUAKHCX5472I.SJP0AWQRYVFA@gonk>
<CUAKIOVISECO.2S7MSQFOP1304@gonk>
<1588058.1690220636@sss.pgh.pa.us>
<CUAO24Z1Y72E.2069AD01EB4LO@gonk>
<CANhcyEVBQWoPJK+C_h1CKyN3Wr2VKHuPx=bSB4PH2LuGdut4qg@mail.gmail.com>
<CWRWDTTPP0WZ.2W75ZAVL5ND1M@neon.tech>
<5e28b89f-0033-4e42-8184-b71b731c1aa2@iki.fi>
<CX5IOLTR8MAD.1UUDEG0X673Z4@neon.tech>
<9b3693b7-0bc3-430a-b43f-eae44e8a39b6@iki.fi>
<CX5NSE8R07D8.9D8M8LBKP119@neon.tech>
<032868e0-a530-4fc7-972c-af6f08950b7e@iki.fi>
<CXBHH2O58HVA.JUP07AES8JVK@neon.tech>
<CXGM82ZEGMR2.8EX2V7UBFIK3@neon.tech>
<CA+TgmoYK1JCJUyrK==TBj7St1hhke5AOwXyrmDAs-pA_mmAwVw@mail.gmail.com>
<CY93J05RFBUI.3VGP5PLTG4EWI@neon.tech>
<CA+TgmoZb1Ojhw9numxWp8nCioh_wULxOcYCQ6Jwku4t_e43PkA@mail.gmail.com>
<CYCWA43RK8AH.11BZB0P45NOD5@neon.tech>
<CYSE2DQI20CH.19LUJSCFH0748@neon.tech>
<CAGECzQTX=esZ+71gAcHGYDBnv65-RGH14xpiFvJytN0uvULzCA@mail.gmail.com>
<CYT3BG48WV2H.25BS2RN173HG9@neon.tech>
<CA+Tgmoa1Uaqnd_OnV=KrQ5qf3=wYv8JM=3SC1e_=0xo0o9KEjg@mail.gmail.com>
On Fri Mar 22, 2024 at 9:59 AM CDT, Robert Haas wrote:
> On Wed, Jan 31, 2024 at 1:07 PM Tristan Partin <tristan@neon.tech> wrote:
> > I was looking for documentation of PQsocket(), but didn't find any
> > standalone (unless I completely missed it). So I just copied how
> > PQsocket() is documented in PQconnectPoll(). I am happy to document it
> > separately if you think it would be useful.
>
> As Jelte said back at the end of January, I think you just completely
> missed it. The relevant part of libpq.sgml starts like this:
>
> <varlistentry id="libpq-PQsocket">
> <term><function>PQsocket</function><indexterm><primary>PQsocket</primary></indexterm></term>
>
> As far as I know, we document all of the exported libpq functions in
> that SGML file.
>
> I think there's no real reason why we couldn't get at least 0001 and
> maybe also 0002 into this release, but only if you move quickly on
> this. Feature freeze is approaching rapidly.
>
> Modulo the documentation changes, I think 0001 is pretty much ready to
> go. 0002 needs comments. I'm also not so sure about the name
> process_connection_state_machine(); it seems a little verbose. How
> about something like wait_until_connected()? And maybe put it below
> do_connect() instead of above.
Robert, Jelte:
Sorry for taking a while to get back to y'all. I have taken your
feedback into consideration for v9. This is my first time writing
Postgres docs, so I'm ready for another round of editing :).
Robert, could you point out some places where comments would be useful
in 0002? I did rename the function and moved it as suggested, thanks! In
turn, I also realized I forgot a prototype, so also added it.
This patchset has also been rebased on master, so the libpq export
number for PQsocketPoll was bumped.
--
Tristan Partin
Neon (https://neon.tech)
Attachments:download all (2 files, .tar.gz)v9-0001-Expose-PQsocketPoll-for-frontends.patch (text/x-patch, 5.9K) download
From 7f8bf7250ecf79f65c129ccc42643c36bc53f882 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Tue, 30 Jan 2024 15:40:57 -0600
Subject: [PATCH v9 1/2] Expose PQsocketPoll for frontends
PQsocketPoll should help with state machine processing when connecting
to a database asynchronously via PQconnectStart().
---
doc/src/sgml/libpq.sgml | 38 +++++++++++++++++++++++++++++++-
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-misc.c | 7 +++---
src/interfaces/libpq/libpq-fe.h | 4 ++++
4 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgmlindex d3e87056f2c..10f191f6b88 100644--- a/doc/src/sgml/libpq.sgml+++ b/doc/src/sgml/libpq.sgml@@ -262,6 +262,39 @@ PGconn *PQsetdb(char *pghost,
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQsocketPoll">+ <term><function>PQsocketPoll</function><indexterm><primary>PQsocketPoll</primary></indexterm></term>+ <listitem>+ <para>+ <indexterm><primary>nonblocking connection</primary></indexterm>+ Poll a connection's underlying socket descriptor retrieved with <xref linkend="libpq-PQsocket"/>.+<synopsis>+int PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time);+</synopsis>+ </para>++ <para>+ This function takes care of the setup for monitoring a file descriptor. The underlying+ function is either <function>poll(2)</function> or <function>select(2)</function>,+ depending on platform support. The primary use of this function is working through the state+ machine instantiated by <xref linkend="libpq-PQconnectStartParams"/>.+ </para>++ <para>+ The function returns a value greater than <literal>0</literal> if the specified condition+ is met, <literal>0</literal> if a timeout occurred, or <literal>-1</literal> if an error+ or interrupt occurred. In the event <literal>forRead</literal> and+ <literal>forWrite</literal> are not set, the function immediately retuns a timeout+ condition.+ </para>++ <para>+ <literal>end_time</literal> is the time in the future in seconds starting from the UNIX+ epoch in which you would like the function to return if the condition is not met.+ </para>+ </listitem>+ </varlistentry>+
<varlistentry id="libpq-PQconnectStartParams">
<term><function>PQconnectStartParams</function><indexterm><primary>PQconnectStartParams</primary></indexterm></term>
<term><function>PQconnectStart</function><indexterm><primary>PQconnectStart</primary></indexterm></term>
@@ -358,7 +391,10 @@ PostgresPollingStatusType PQconnectPoll(PGconn *conn);
Loop thus: If <function>PQconnectPoll(conn)</function> last returned
<symbol>PGRES_POLLING_READING</symbol>, wait until the socket is ready to
read (as indicated by <function>select()</function>, <function>poll()</function>, or
- similar system function).+ similar system function). Note that <function>PQsocketPoll</function>+ can help reduce boilerplate by abstracting the setup of+ <function>select(2)</function> or <function>poll(2)</function> if it is+ available on your system.
Then call <function>PQconnectPoll(conn)</function> again.
Conversely, if <function>PQconnectPoll(conn)</function> last returned
<symbol>PGRES_POLLING_WRITING</symbol>, wait until the socket is ready
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txtindex 9fbd3d34074..1e48d37677d 100644--- a/src/interfaces/libpq/exports.txt+++ b/src/interfaces/libpq/exports.txt@@ -202,3 +202,4 @@ PQcancelSocket 199
PQcancelErrorMessage 200
PQcancelReset 201
PQcancelFinish 202
+PQsocketPoll 203diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.cindex f2fc78a481c..f562cd8d344 100644--- a/src/interfaces/libpq/fe-misc.c+++ b/src/interfaces/libpq/fe-misc.c@@ -55,7 +55,6 @@ static int pqPutMsgBytes(const void *buf, size_t len, PGconn *conn);
static int pqSendSome(PGconn *conn, int len);
static int pqSocketCheck(PGconn *conn, int forRead, int forWrite,
time_t end_time);
-static int pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time);
/*
* PQlibVersion: return the libpq version number
@@ -1059,7 +1058,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
/* We will retry as long as we get EINTR */
do
- result = pqSocketPoll(conn->sock, forRead, forWrite, end_time);+ result = PQsocketPoll(conn->sock, forRead, forWrite, end_time);
while (result < 0 && SOCK_ERRNO == EINTR);
if (result < 0)
@@ -1083,8 +1082,8 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
* Timeout is infinite if end_time is -1. Timeout is immediate (no blocking)
* if end_time is 0 (or indeed, any time before now).
*/
-static int-pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)+int+PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time)
{
/* We use poll(2) if available, otherwise select(2) */
#ifdef HAVE_POLL
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.hindex 09b485bd2bc..98c27415281 100644--- a/src/interfaces/libpq/libpq-fe.h+++ b/src/interfaces/libpq/libpq-fe.h@@ -21,6 +21,7 @@ extern "C"
#endif
#include <stdio.h>
+#include <time.h>
/*
* postgres_ext.h defines the backend's externally visible types,
@@ -670,6 +671,9 @@ extern int lo_export(PGconn *conn, Oid lobjId, const char *filename);
/* Get the version of the libpq library in use */
extern int PQlibVersion(void);
+/* Poll a file descriptor for reading and/or writing with a timeout */+extern int PQsocketPoll(int sock, int forRead, int forWrite, time_t end_time);+
/* Determine length of multibyte encoded char at *s */
extern int PQmblen(const char *s, int encoding);
--
Tristan Partin
Neon (https://neon.tech)
v9-0002-Allow-SIGINT-to-cancel-psql-database-reconnection.patch (text/x-patch, 2.8K) download
From 779e642df8013b3fc8c63f9faf370cbbf2950392 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Tue, 30 Jan 2024 15:53:10 -0600
Subject: [PATCH v9 2/2] Allow SIGINT to cancel psql database reconnections
After installing the SIGINT handler in psql, SIGINT can no longer cancel
database reconnections. For instance, if the user starts a reconnection
and then needs to do some form of interaction (ie psql is polling),
there is no way to cancel the reconnection process currently.
Use PQconnectStartParams() in order to insert a cancel_pressed check
into the polling loop.
---
src/bin/psql/command.c | 50 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.cindex 9b0fa041f73..00a20e47328 100644--- a/src/bin/psql/command.c+++ b/src/bin/psql/command.c@@ -159,6 +159,7 @@ static void discard_query_text(PsqlScanState scan_state, ConditionalStack cstack
static bool copy_previous_query(PQExpBuffer query_buf, PQExpBuffer previous_buf);
static bool do_connect(enum trivalue reuse_previous_specification,
char *dbname, char *user, char *host, char *port);
+static void wait_until_connected(PGconn *conn);
static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
int lineno, bool discard_on_quit, bool *edited);
static bool do_shell(const char *command);
@@ -3595,11 +3596,12 @@ do_connect(enum trivalue reuse_previous_specification,
values[paramnum] = NULL;
/* Note we do not want libpq to re-expand the dbname parameter */
- n_conn = PQconnectdbParams(keywords, values, false);+ n_conn = PQconnectStartParams(keywords, values, false);
pg_free(keywords);
pg_free(values);
+ wait_until_connected(n_conn);
if (PQstatus(n_conn) == CONNECTION_OK)
break;
@@ -3748,6 +3750,52 @@ do_connect(enum trivalue reuse_previous_specification,
return true;
}
+static void+wait_until_connected(PGconn *conn)+{+ bool for_read = false;++ while (true)+ {+ int rc;+ int sock;+ time_t timeout;++ /*+ * On every iteration of the state machine, let's check if the user has+ * requested a cancellation of the connection process.+ */+ if (cancel_pressed)+ break;++ sock = PQsocket(conn);+ if (sock == -1)+ break;++ /* Use a 1 second timeout */+ timeout = time(NULL) + 1;+ rc = PQsocketPoll(sock, for_read, !for_read, timeout);+ if (rc == -1)+ return;++ switch (PQconnectPoll(conn))+ {+ case PGRES_POLLING_OK:+ case PGRES_POLLING_FAILED:+ return;+ case PGRES_POLLING_READING:+ for_read = true;+ continue;+ case PGRES_POLLING_WRITING:+ for_read = false;+ continue;+ case PGRES_POLLING_ACTIVE:+ pg_unreachable();+ }+ }++ pg_unreachable();+}
void
connection_warnings(bool in_startup)
--
Tristan Partin
Neon (https://neon.tech)
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: pgsql-hackers@postgresql.org
Cc: tristan@neon.tech, robertmhaas@gmail.com, postgres@jeltef.nl, hlinnaka@iki.fi, tgl@sss.pgh.pa.us, shlok.kyal.oss@gmail.com
Subject: Re: psql not responding to SIGINT upon db reconnection
In-Reply-To: <D00FXVDVQG85.1CBNG8IZ2XM8E@neon.tech>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox