On Fri Mar 22, 2024 at 9:59 AM CDT, Robert Haas wrote:
On Wed, Jan 31, 2024 at 1:07 PM Tristan Partin <tris...@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)
From 7f8bf7250ecf79f65c129ccc42643c36bc53f882 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@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.sgml
index 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.txt
index 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 203
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 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.h
index 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)
From 779e642df8013b3fc8c63f9faf370cbbf2950392 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@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.c
index 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)
