RE: Timeout parameters

Nagaura, Ryohei <nagaura.ryohei@jp.fujitsu.com>

From: "Nagaura, Ryohei" <nagaura.ryohei@jp.fujitsu.com>
To: "Jamison, Kirk" <k.jamison@jp.fujitsu.com>, "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>
Cc: 'Kyotaro HORIGUCHI' <horiguchi.kyotaro@lab.ntt.co.jp>, "coelho@cri.ensmp.fr" <coelho@cri.ensmp.fr>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "MikalaiKeida@ibagroup.eu" <MikalaiKeida@ibagroup.eu>, "AYahorau@ibagroup.eu" <AYahorau@ibagroup.eu>, "michael@paquier.xyz" <michael@paquier.xyz>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2019-03-29T04:39:37Z
Lists: pgsql-hackers

Attachments

Hi, Tsunakawa-san, Kirk-san.

Thank you for your review.


Tsunakawa-san,
> From: Tsunakawa, Takayuki [mailto:tsunakawa.takay@jp.fujitsu.com]
> The client-side tcp_user_timeout patch looks good.
Thanks, but I minorly changed that patch.
It is the declaration place of sebuf[], moving to just before where it'll become needed.


> The server-side tcp_user_timeout patch needs fixing the following:
> (1)
> +			GUC_UNIT_MS | GUC_NOT_IN_SAMPLE
> +		12000, 0, INT_MAX,
> 
> GUC_NOT_IN_SAMPLE should be removed because the parameter appears in 
> postgresql.conf.sample.
> The default value should be 0, not 12000.
Oops, fixed!

> (2)
> Now that you've changed show_tcp_usertimeout() to use 
> pq_gettcpusertimeout(), you need to modify pq_settcpusertimeout(); just immitate pq_setkeepalivesidle().
> Otherwise, SHOW would display a wrong value.
Indeed, pq_settcpusertimeout() should be modified as well.


> The socket_timeout patch needs the following fixes.  Now that others 
> have already tested these patches successfully, they appear committable to me.
Thank you so much for reviewing even socket_timeout patch.

> The reason why oom_error label is present is that it is used at 
> multiple places to avoid repeating the same error processing code.
Understood.

> (2)
> +			conn->sock = -1;
> 
> Use PGINVALID_SOCKET instead of -1.
Oh, I see.


Kirk-san,
> I referred to the doc in libpq.sgml, corrected misspellings, and rephrased the
> doc a little bit as below:
Changed "stop-gap" and "mean" -> "stopgap" and "measure" respectively.
But I can't find the difference between mine and yours.
Would you please tell me if there is a difference like this depending on your rephrasing?

Best regards,
---------------------
Ryohei Nagaura


Commits

  1. Add support TCP user timeout in libpq and the backend server