Re: Add client connection check during the execution of the query

Hi,

On 2019-01-13 18:05:39 -0500, Tom Lane wrote:
> s.cherkashin@postgrespro.ru writes:
> > This patch adds verification of the connection with the client during 
> > the execution of the SQL query. The feature enables using the GUC 
> > variable ‘client_connection_check_interval’. The default check interval 
> > is 1 second. If you set the value of ‘client_connection_check_interval’ 
> > to 0, then the check will not be performed.
> 
> I took a quick look through this.
> 
> * It won't even compile on non-Linux platforms, because MSG_DONTWAIT
> is a Linux-ism.  Perhaps that can be replaced by putting the client
> socket into nonblock mode, but I'm not very sure that that'll work
> (especially when using OpenSSL or another TLS implementation).
> 
> * I'm not convinced that this will reliably detect client connection loss.
> AFAICS, if there is any unread data pending, it'd report that all is well
> even if the client dropped off the net after sending that data.  It's hard
> to evaluate how likely such a situation is, but one really obvious case
> is that the client might've sent an 'X' message to try to close the
> connection gracefully.  Also, use of TLS would again make things much
> harder to reason about, because the TLS layer may send or receive data
> that we don't know about.
> 
> * The management of the pending timeout interrupt seems like a mess.
> Why did you involve ProcessInterrupts in that?  It seems likely to queue
> extra timeouts at random times due to unrelated interrupts causing that
> bit of code to run, and/or cause weird gaps in the timeout intervals due
> to not being run promptly.  I'd be inclined to set this up so that the
> timeout handler itself re-queues the timeout (I think that will work, or
> if not, we should probably fix timeout.c so that it can).
> 
> * BTW, I am not on board with making this enabled-by-default.
> 
> This does seem like possibly a useful option if we can make it
> work portably/reliably, but I don't have very high hopes for that.

Given that nothing happened since this message, and the commitfest is
ending, I'm going to mark this as returned with feedback.

Greetings,

Andres Freund