Re: [PATCH] Prevent hanging on unreachable hosts on startup

On Thu, Jan 5, 2012 at 09:23:55AM -0500, Ryan Kelly wrote:
> On Wed, Jan 04, 2012 at 09:36:57PM -0500, Tom Lane wrote:
> > "Ryan P. Kelly" <rpkelly22(at)gmail(dot)com> writes:
> > > The signal handler installed by setup_cancel_handler() will ignore
> > > attempts to exit psql should a host be unreachable.
> >
> > Hm. That may be worth doing something about, but the proposed patch
> > seems more like a quick-and-dirty hack than a solution. The main
> > thing I don't like about it is that if we care about this scenario
> > during initial startup, we should also care about it during a \c
> > command, but just delaying the installation of the signal handler
> > won't fix that case.
> Sure, but do you still think moving the signal handler down is correct?
> I've attached another patch that handles interrupting \c. I also noticed
> there appears to be what looks like duplication of code in startup.c and
> command.c in main() and do_connect() respectively. I'm wondering if I
> they should be made to share the same code which (with my patch) would
> then mean the signal handler could be left where it was.
>
> > More generally, what if the server becomes unreachable mid-session?
> > I'm not sure how much there is to be done about that case, since
> > there's probably no good way to distinguish it from a query that
> > takes a really long time. But if we're going to think about this,
> > we might as well think about all the cases.
> Well in this case we can still interrupt the query, no? This will drop
> us back into the prompt, where we can safely \q.

Tom, can you comment on this patch because you commented on the previous
version? Thanks.

---------------------------------------------------------------------------

> >From 8f9d0b5088021d944aceac65a96d7bd0c24aa0c6 Mon Sep 17 00:00:00 2001
> From: "Ryan P. Kelly" <rpkelly22(at)gmail(dot)com>
> Date: Thu, 5 Jan 2012 09:13:38 -0500
> Subject: [PATCH] Allow interrupting hanging \c connection attempts
>
> ---
> src/bin/psql/command.c | 17 +++++++++++++----
> 1 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
> index 69fac83..845705d 100644
> --- a/src/bin/psql/command.c
> +++ b/src/bin/psql/command.c
> @@ -1516,7 +1516,7 @@ static bool
> do_connect(char *dbname, char *user, char *host, char *port)
> {
> PGconn *o_conn = pset.db,
> - *n_conn;
> + *n_conn = NULL;
> char *password = NULL;
>
> if (!dbname)
> @@ -1570,7 +1570,13 @@ do_connect(char *dbname, char *user, char *host, char *port)
> keywords[7] = NULL;
> values[7] = NULL;
>
> - n_conn = PQconnectdbParams(keywords, values, true);
> + if (sigsetjmp(sigint_interrupt_jmp, 1) != 0) {
> + /* got here with longjmp */
> + } else {
> + sigint_interrupt_enabled = true;
> + n_conn = PQconnectdbParams(keywords, values, true);
> + sigint_interrupt_enabled = false;
> + }
>
> free(keywords);
> free(values);
> @@ -1600,7 +1606,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
> */
> if (pset.cur_cmd_interactive)
> {
> - psql_error("%s", PQerrorMessage(n_conn));
> + if (n_conn)
> + psql_error("%s", PQerrorMessage(n_conn));
>
> /* pset.db is left unmodified */
> if (o_conn)
> @@ -1608,7 +1615,9 @@ do_connect(char *dbname, char *user, char *host, char *port)
> }
> else
> {
> - psql_error("\\connect: %s", PQerrorMessage(n_conn));
> + if (n_conn)
> + psql_error("\\connect: %s", PQerrorMessage(n_conn));
> +
> if (o_conn)
> {
> PQfinish(o_conn);
> --
> 1.7.7.4
>

>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +