From: Tristan Partin <tristan@neon.tech>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: psql not responding to SIGINT upon db reconnection
Date: Mon, 24 Jul 2023 14:57:58 -0500
Message-ID: <CUAO24Z1Y72E.2069AD01EB4LO@gonk> (raw)
In-Reply-To: <1588058.1690220636@sss.pgh.pa.us>
References: <CUAJJDVD8L61.2AH6VEEDMTXX3@gonk>
<CUAKHCX5472I.SJP0AWQRYVFA@gonk>
<CUAKIOVISECO.2S7MSQFOP1304@gonk>
<1588058.1690220636@sss.pgh.pa.us>
On Mon Jul 24, 2023 at 12:43 PM CDT, Tom Lane wrote:
> "Tristan Partin" <tristan@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)
Attachments:v4-0001-Allow-SIGINT-to-cancel-psql-database-reconnection.patch (text/x-patch, 1.9K) download
From 73a95bc537f205cdac567f3f1860b08cf8323e17 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@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.cindex 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)
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, tgl@sss.pgh.pa.us
Subject: Re: psql not responding to SIGINT upon db reconnection
In-Reply-To: <CUAO24Z1Y72E.2069AD01EB4LO@gonk>
* 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