RE: Timeout parameters

From: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
To: "Jamison, Kirk" <k(dot)jamison(at)jp(dot)fujitsu(dot)com>, "Nagaura, Ryohei" <nagaura(dot)ryohei(at)jp(dot)fujitsu(dot)com>
Cc: 'Michael Paquier' <michael(at)paquier(dot)xyz>, "AYahorau(at)ibagroup(dot)eu" <AYahorau(at)ibagroup(dot)eu>, 'Fabien COELHO' <coelho(at)cri(dot)ensmp(dot)fr>, "'pgsql-hackers(at)postgresql(dot)org'" <pgsql-hackers(at)postgresql(dot)org>, "MikalaiKeida(at)ibagroup(dot)eu" <MikalaiKeida(at)ibagroup(dot)eu>
Subject: RE: Timeout parameters
Date: 2019-02-22 00:45:54
Message-ID: 0A3221C70F24FB45833433255569204D1FB9F46E@G01JPEXMBYT05
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

From: Jamison, Kirk/ジャミソン カーク
> Although I did review and followed the suggested way in previous email
> way back (which uses root user) and it worked as intended, I'd also like
> to hear feedback also from Fabien whether it's alright without the test
> script, or if there's another way we can test it (maybe none?).
> Then I think it's in good shape.

I'm afraid there's not a way to incorporate the test into the regression test. If there is, Fabien should have responded earlier.

> However, what I meant by my statement above was to have a separate
> decision in committing the two parameters, because based from the thread
> tcp_user_timeout has got more progress when it comes to reviews.
> So once it's ready, it can be committed separately without waiting for
> the socket_timeout param to be ready. (I dont think that requires a
> separate thread)

I see, thanks.

> As for the place, my knowledge is limited so I won't be able to provide
> substantial help on it. Fabien pointed out some comments about it that needs
> clarification though.
> Quoting what Fabien wrote:
> "The implementation looks awkward, because part of the logic of
> pqWaitTimed
> is reimplemented in pqWait. Also, I do not understand the
> computation
> of finish_time, which seems to assume that pqWait is going to be
> called
> immediately after sending a query, which may or may not be the case,
> and
> if it is not the time() call there is not the start of the statement."

The patch looks good in that respect. This timeout is for individual socket operations like PgJDBC and Oracle, not the query timeout. That's what the parameter name suggests.

> How about the one below?
> I got it from combined explanations above. I did not include the
> differences though. This can be improved as the discussion
> continues along the way.
>
> tcp_socket_timeout (integer)
>
> Terminate and restart any session that has been idle for more than
> the specified number of milliseconds to prevent client from infinite
> waiting for server due to dead connection. This can be used both as
> a brute force global query timeout and detecting network problems.
> A value of zero (the default) turns this off.

Looks better than the original one. However,

* The unit is not millisecond, but second.
* Doesn't restart the session.
* Choose words that better align with the existing libpq parameters. e.g. "connection" should be used instead of "session" as in keepalive_xxx parameters, because there's not a concept of database session at socket level.
* Indicate that the timeout is for individual socket operations.

Regards
Takayuki Tsunakawa

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Iwata, Aya 2019-02-22 00:52:09 RE: libpq debug log
Previous Message Chapman Flack 2019-02-22 00:34:50 Re: proposal: variadic argument support for least, greatest function