Lists: | pgsql-hackers |
---|
From: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
---|---|
To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | psql sets up cancel handler very early |
Date: | 2013-05-14 15:35:36 |
Message-ID: | 519259C8.6060401@gmx.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Sometimes, the psql startup hangs when it cannot resolve or connect to a
host. Intuitively, I would like to press Ctrl+C and correct the
connection string or investigate. But that doesn't work because Ctrl+C
is already bound to the query cancel handler by that time.
It seems to me that there is no point in setting up the cancel handler
before the database connection is established. Example patch attached.
Comments?
Attachment | Content-Type | Size |
---|---|---|
psql-startup-cancel-handler.patch | text/plain | 568 bytes |
From: | Ryan Kelly <rpkelly22(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: psql sets up cancel handler very early |
Date: | 2013-05-14 15:41:49 |
Message-ID: | 20130514154149.GD15128@llserver.lakeliving.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
I submitted essentially this same patch over a year ago and Tom vetoed
it: http://www.postgresql.org/message-id/3741.1325731017@sss.pgh.pa.us
The thread moved to -hackers at some point and I made some further
enhancements: http://www.postgresql.org/message-id/20120108201802.GA31348@llserver.lakeliving.com
Never went anywhere though.
-Ryan P. Kelly
On Tue, May 05/14/13, 2013 at 11:35:36AM -0400, Peter Eisentraut wrote:
> Sometimes, the psql startup hangs when it cannot resolve or connect to a
> host. Intuitively, I would like to press Ctrl+C and correct the
> connection string or investigate. But that doesn't work because Ctrl+C
> is already bound to the query cancel handler by that time.
>
> It seems to me that there is no point in setting up the cancel handler
> before the database connection is established. Example patch attached.
> Comments?
> diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
> index 5d7fe6e..65a1cde 100644
> --- a/src/bin/psql/startup.c
> +++ b/src/bin/psql/startup.c
> @@ -111,8 +111,6 @@ static void parse_psql_options(int argc, char *argv[],
> setvbuf(stderr, NULL, _IONBF, 0);
> #endif
>
> - setup_cancel_handler();
> -
> pset.progname = get_progname(argv[0]);
>
> pset.db = NULL;
> @@ -249,6 +247,8 @@ static void parse_psql_options(int argc, char *argv[],
> exit(EXIT_BADCONN);
> }
>
> + setup_cancel_handler();
> +
> PQsetNoticeProcessor(pset.db, NoticeProcessor, NULL);
>
> SyncVariables();
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: psql sets up cancel handler very early |
Date: | 2013-09-18 11:45:24 |
Message-ID: | CAEZATCUifDD9HBH-49TBKmJ9k6_D7Occ8B+SV1=f5JhggRsSjA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On 14 May 2013 16:35, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Sometimes, the psql startup hangs when it cannot resolve or connect to a
> host. Intuitively, I would like to press Ctrl+C and correct the
> connection string or investigate. But that doesn't work because Ctrl+C
> is already bound to the query cancel handler by that time.
>
> It seems to me that there is no point in setting up the cancel handler
> before the database connection is established. Example patch attached.
> Comments?
>
That makes sense to me, and the patch appears to work as advertised.
The objections to the previous patch were that it did nothing in the
\c case, or if the server becomes unreachable mid-session. Those feel
like much less common cases, but maybe they're still worth thinking
about. However, IMO the solution to those issues is likely to be a
significantly different (and larger) patch.
Also, even if those issues do get addressed one day, the change in
this patch still seems like the right thing to do on initial startup,
which IME is the most common case, so +1 for this patch.
I'm marking it ready for committer.
Regards,
Dean