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

For v4-0002-some-fixups.patch :

+ if (client_connection_check_interval > 0)
+ enable_timeout_after(CLIENT_CONNECTION_CHECK_TIMEOUT,

+ /* Start timeout for checking if the client has gone away if necessary.
*/
+ if (client_connection_check_interval)

It would be better if the second if condition is aligned with that of the
first (> 0).

For v4-0001-Detect-dropped-connections-while-running-queries.patch :

+ Sets a time interval, in milliseconds, between periodic

I wonder if the interval should be expressed in seconds. Since the
description says: while running very long queries.

Cheers

On Fri, Mar 5, 2021 at 8:07 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:

> On Mon, Mar 1, 2021 at 6:18 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
> wrote:
> > I've done a quick rebase of this the patch and added it to the
> > commitfest. No other changes. Several things were mentioned earlier
> > that still need to be tidied up.
>
> Rebased again due to bitrot. This time I did some actual work:
>
> 1. I didn't like the way it was rearming the timer *in the timer
> handler*; I think it should be done in the CFI(), and only if it
> determines that you're still running a query (otherwise you'll get
> periodic wakeups while you're idle between quieries, which is bad for
> the arctic ice cap; we already handle client going away efficiently
> between queries with WaitEventSet socket readiness).
> 2. The timer handler surely has to set the latch to close a race (cf.
> other similar handlers; between the CFI() and the beginning of the
> sleep, you could handle the signal, set the flag, and then go to sleep
> for 100 years).
> 3. The test might as well use pg_sleep() instead of doing a plpgsql
> busy loop of SELECT queries.
> 4. I prefer the name CLIENT_CONNECTION_CHECK_TIMEOUT instead of
> SKIP_CLIENT_CHECK_TIMEOUT; let's make up only one new name for a
> concept instead of two.
> 5. Miniscule doc change.
>
> I put these into a separate patch for ease of review. I don't claim
> this is ready -- still needs more testing etc -- but it seems to be
> generating the right system calls at the right times now.
>