Re: [HACKERS] psql casts aspersions on server reliability

On Thu, 2023-11-23 at 11:12 -0500, Bruce Momjian wrote:
> On Wed, Nov 22, 2023 at 10:25:14PM -0500, Bruce Momjian wrote:
> > Yes, you are correct.  Here is a patch that implements the FATAL test,
> > though I am not sure I have the logic correct or backwards, and I don't
> > know how to test this.  Thanks.
>
> I developed the attached patch which seems to work better.  In testing
> kill -3 on a backend or calling elog(FATAL) in the server for a
> session, libpq's 'res' is NULL, meaning we don't have any status to
> check for PGRES_FATAL_ERROR.  It is very possible that libpq just isn't
> stuctured to have the PGRES_FATAL_ERROR at the point where we issue this
> message, and this is not worth improving.
>
>     test=> select pg_sleep(100);
> -->    FATAL:  FATAL called
>
>     server closed the connection unexpectedly
> -->            This probably means the server terminated null
>             before or while processing the request.
>     The connection to the server was lost. Attempting reset: Succeeded.

I don't thing "terminated null" is a meaningful message.

>      libpq_append_conn_error(conn, "server closed the connection unexpectedly\n"
> -                            "\tThis probably means the server terminated abnormally\n"
> -                            "\tbefore or while processing the request.");
> +                            "\tThis probably means the server terminated%s\n"
> +                            "\tbefore or while processing the request.",
> +                            (conn->result == NULL) ? " null" :
> +                            (conn->result->resultStatus == PGRES_FATAL_ERROR) ?
> +                            "" : " abnormally");

Apart from the weird "null", will that work well for translation?

> --- a/src/interfaces/libpq/fe-protocol3.c
> +++ b/src/interfaces/libpq/fe-protocol3.c
> @@ -2158,6 +2158,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid,
>                  if (pqGetErrorNotice3(conn, true))
>                      continue;
>                  status = PGRES_FATAL_ERROR;
> +                fprintf(stderr, "Got 'E'\n");
>                  break;
>              case 'A':            /* notify message */
>                  /* handle notify and go back to processing return values */

That looks like a leftover debugging message.

Yours,
Laurenz Albe