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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: s(dot)cherkashin(at)postgrespro(dot)ru, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add client connection check during the execution of the query
Date: 2019-01-31 13:56:08
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2019-01-13 18:05:39 -0500, Tom Lane wrote:
> s(dot)cherkashin(at)postgrespro(dot)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.


Andres Freund

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Yugo Nagata 2019-01-31 14:20:32 Re: Implementing Incremental View Maintenance
Previous Message Daniel Gustafsson 2019-01-31 13:55:53 Re: Allow auto_explain to log to NOTICE