On Mon Jul 24, 2023 at 12:43 PM CDT, Tom Lane wrote:
"Tristan Partin" <tris...@neon.tech> writes:
> v3 is attached which fixes up some code comments I added which I hadn't
> attached to the commit already, sigh.
I don't care for this patch at all. You're bypassing the pqsignal
abstraction layer that the rest of psql goes through, and the behavior
you're implementing isn't very nice. People do not expect ^C to
kill psql - it should just stop the \c attempt and leave you as you
were.
Admittedly, getting PQconnectdbParams to return control on SIGINT
isn't too practical. But you could probably replace that with a loop
around PQconnectPoll and test for CancelRequested in the loop.
That sounds like a much better solution. Attached you will find a v4
that implements your suggestion. Please let me know if there is
something that I missed. I can confirm that the patch works.
$ ./build/src/bin/psql/psql -h pg.neon.tech
NOTICE: Welcome to Neon!
Authenticate by visiting:
https://console.neon.tech/psql_session/xxx
NOTICE: Connecting to database.
psql (17devel, server 15.3)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384,
compression: off)
Type "help" for help.
tristan957=> \c
NOTICE: Welcome to Neon!
Authenticate by visiting:
https://console.neon.tech/psql_session/yyy
^Cconnection to server at "pg.neon.tech" (3.18.6.96), port 5432 failed:
Previous connection kept
tristan957=>
--
Tristan Partin
Neon (https://neon.tech)
From 73a95bc537f205cdac567f3f1860b08cf8323e17 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Mon, 24 Jul 2023 11:12:59 -0500
Subject: [PATCH v4] 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 CancelRequested check
into the polling loop.
---
src/bin/psql/command.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 6733f008fd..bfecc25801 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -40,6 +40,7 @@
#include "large_obj.h"
#include "libpq-fe.h"
#include "libpq/pqcomm.h"
+#include "libpq/pqsignal.h"
#include "mainloop.h"
#include "portability/instr_time.h"
#include "pqexpbuffer.h"
@@ -3577,11 +3578,33 @@ 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);
+ while (true) {
+ int socket;
+
+ if (CancelRequested)
+ break;
+
+ socket = PQsocket(n_conn);
+ if (socket == -1)
+ break;
+
+ switch (PQconnectPoll(n_conn)) {
+ case PGRES_POLLING_OK:
+ case PGRES_POLLING_FAILED:
+ break;
+ case PGRES_POLLING_READING:
+ case PGRES_POLLING_WRITING:
+ continue;
+ case PGRES_POLLING_ACTIVE:
+ pg_unreachable();
+ }
+ }
+
if (PQstatus(n_conn) == CONNECTION_OK)
break;
--
Tristan Partin
Neon (https://neon.tech)
