[PATCH] immediately kill psql process if server is not running.

On 11/8/24 19:42, Srinath Reddy Sadipiralla wrote:

Hi ,

When a user tries to enter a large query, thinking the server is still
running, and to their surprise, they discover that the server has
crashed or stopped, I have observed that the psql process will still be
running if it is idle even after the server crashes or stops.

solved this using 2 methods:

1) using a thread (supports --with-readline and --without-readline)

To address this surprise circumstance, I created a patch that creates a
new thread within the psql process to monitor the File Descriptor of the
psql process,this thread will monitor for -1 or 0 on psql’s FD using
select() and recv()  functions .If either one of the value is returned
we terminate the psql process.

2) Doing the same thing as above but in single process using readline
callbacks(only supports --with-readline)

Yes, it's true that if the connection breaks, the client may not notice
that until the next command. But why would we want to kill psql in that
case? How does that improve the user experience?

If you're in an interactive session (which is where this matters), and
you're not running any commands, you're most likely not paying any
attention to it. So if psql terminates, you won't notice that either. It
doesn't change anything.

Also, it's a long-standing behavior that it the connection closes, I can
simply repeat the query and it's automatically re-opened.

test=# select 1;
FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
test=# select 1;
?column?
----------
1
(1 row)

If we just kill psql instead, this won't work anymore. Why would that be
better?

Also, just killing the process (no matter if by kill or exit), this
means the usual cleanup doesn't happen. So stuff is not written to
.psql_history, for example. That doesn't seem great.

Overall, I don't quite see why the current behavior is a problem, and/or
why would this be an improvement.

thanks

--
Tomas Vondra

Tomas Vondra <tomas@vondra.me> writes:

Yes, it's true that if the connection breaks, the client may not notice
that until the next command. But why would we want to kill psql in that
case? How does that improve the user experience?

If you're in an interactive session (which is where this matters), and
you're not running any commands, you're most likely not paying any
attention to it. So if psql terminates, you won't notice that either. It
doesn't change anything.

Yeah, I concur that this isn't an improvement. In addition to the
points Tomas raises, consider the possibility that you're typing into
a psql session and not watching closely, and the patch causes psql
to quit asynchronously. Now you are typing at a shell prompt.
If you still aren't watching and hit return, unpleasant results
might ensue.

Perhaps there's something that we could do here, but I'm not sure
what would be a safe behavioral change. We've generally avoided
allowing psql to take actions asynchronously --- as an example,
it doesn't report NOTIFY or NOTICE events during user command input,
even though they might be quite old by the time they do get reported.

One idea that seems like it could be safe is to change the prompt,
so that your experience could be like

postgres=# SELECT foo, bar, baz, <enter>
<DEAD!>-# (uh-oh)

This only helps for people who are entering multi-line SQL, but
perhaps the most aggravating form of this is where you spend awhile
crafting a long command only to see it fail. On the other hand,
as Tomas says, up-arrow and retry works regardless of command
length. So maybe it's not worth the trouble.

Anyway, I'm inclined to close this CF entry as returned-with-feedback.
Perhaps there's something we can do here, but the current patch feels
like a dead end.

regards, tom lane

I wrote:

Anyway, I'm inclined to close this CF entry as returned-with-feedback.
Perhaps there's something we can do here, but the current patch feels
like a dead end.

Also, the submitter seems to have disappeared:

----- The following addresses had permanent fatal errors -----
<srinath.reddy@zohocorp.com>
(reason: 550 5.1.1 <srinath.reddy@zohocorp.com> User unknown)

----- Transcript of session follows -----
... while talking to mx.zohocorp.com.:

RCPT To:<srinath.reddy@zohocorp.com>

<<< 550 5.1.1 <srinath.reddy@zohocorp.com> User unknown
550 5.1.1 <srinath.reddy@zohocorp.com>... User unknown

DATA

<<< 503 No recipients specified
451 4.4.1 reply: read error from mx.zohocorp.com.

So I'm not going to hold my breath waiting for a reply.

regards, tom lane

On Sun, Dec 8, 2024 at 4:31 AM Tomas Vondra <tomas@vondra.me> wrote:

Yes, it's true that if the connection breaks, the client may not notice
that until the next command. But why would we want to kill psql in that
case? How does that improve the user experience?

Hi Tomas,sorry for the (very!) late reply ,I almost lost hope waiting for
response to my thread :)
sequence of events:
1)server goes down
2)psql still running
3)user is unaware about the server and enters a query
4)boom for a surprise user came to know "server is down"

if in the very 1st place if we killed the psql process if server goes down
,the user won't get the "surprise" that the server has crashed right?,pls
correct me if i am wrong.

Also, it's a long-standing behavior that it the connection closes, I can

simply repeat the query and it's automatically re-opened.

test=# select 1;
FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
test=# select 1;
?column?
----------
1
(1 row)

If we just kill psql instead, this won't work anymore. Why would that be
better?

agreed that psql tries to reconnect but only if the user re-tries entering
the query and it's true if the server starts up immediately.

Also, just killing the process (no matter if by kill or exit), this
means the usual cleanup doesn't happen. So stuff is not written to
.psql_history, for example. That doesn't seem great.

I am assuming cleanup means you are talking about the one which happens at

the end of main where PQfinish,fclose which has already been handled in
this patch.
But as you mentioned, history is not written to .psql_history in this patch
,we can save to psql_history by calling finishInput() ,for this I updated
the patch.

Thanks for the feedback Tomas.

Srinath Reddy Sadipiralla,

On Sun, Jan 19, 2025 at 3:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

One idea that seems like it could be safe is to change the prompt,
so that your experience could be like

postgres=# SELECT foo, bar, baz, <enter>
<DEAD!>-# (uh-oh)

This only helps for people who are entering multi-line SQL,

+1,will look into it.

Thanks.
Srinath Reddy Sadipiralla,