From: | "Nagaura, Ryohei" <nagaura(dot)ryohei(at)jp(dot)fujitsu(dot)com> |
---|---|
To: | 'Kyotaro HORIGUCHI' <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | "coelho(at)cri(dot)ensmp(dot)fr" <coelho(at)cri(dot)ensmp(dot)fr>, "Jamison, Kirk" <k(dot)jamison(at)jp(dot)fujitsu(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, "robertmhaas(at)gmail(dot)com" <robertmhaas(at)gmail(dot)com>, "MikalaiKeida(at)ibagroup(dot)eu" <MikalaiKeida(at)ibagroup(dot)eu>, "AYahorau(at)ibagroup(dot)eu" <AYahorau(at)ibagroup(dot)eu>, "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | RE: Timeout parameters |
Date: | 2019-03-28 08:36:10 |
Message-ID: | EDA4195584F5064680D8130B1CA91C4540F159@G01JPEXMBYT04 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello, Horiguchi-san.
Thank you for review.
> In TCP_backend patch:
> I think this is not mentioning backend. Why don't you copy'n paste then
> modify the description of tcp_keepalives_idle? Perhaps it needs a similar
> caveat related to Windows.
> +static const char*
> +show_tcp_user_timeout(void)
> The reason for, say, tcp_keepalive_idle uses the hook is that it doesn't
> show the GUC variable, but the actual value read by getsockopt. This is
> just showing the GUC value. I think this should behave the same way with
> tcp_keepalive* options. If not, we should have an explanation of the
> reason there.
Oh, Thank you for teaching.
I add a function pq_gettcpusertimeout() same as keepalives*.
> In TCP_interface patch:
> I would suggest the same thing as the backend part. Copy'n-paste-modify
> keepalives_idle would be better.
Same as backend-side, I made keepalives documentation reference.
I refered keepalives* documentation and modified my doc.
> I suppose that the reason ENOPROTOOPT is excluded from error condition
> is that the system call may fail with that errno on older kernels, but
> I don't think that that justifies ignoring the failure.
Thank you for your point!
Removed in both patches.
> I don't see a point in the added part having own "if (!IS_AF_UNIX" block
> separately from tcp_keepalive options.
Sorry, my coding was bad...
I integrated it with coding about keepalives.
> + /* TCP USER TIMEOUT */
> + {"tcp_user_timeout", NULL, NULL, NULL,
> + "TCP_user_timeout", "", 10, /* strlen(INT32_MAX) ==
> 10 */
> + offsetof(struct pg_conn, pgtcp_user_timeout)},
> +
>
> Similary, why isn't this placed just after tcp_keepalives options?
Moved!
> + char *tcp_user_timeout; /* tcp user timeout */
> The latter doesn't seem to be used.
This is also my bad coding...
Removed!
Best regards,
---------------------
Ryohei Nagaura
Attachment | Content-Type | Size |
---|---|---|
TCP_backend_v14.patch | application/octet-stream | 7.2 KB |
TCP_interface_v13.patch | application/octet-stream | 3.7 KB |
socket_timeout_v9.patch | application/octet-stream | 5.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Christoph Berg | 2019-03-28 08:38:16 | Re: Enable data checksums by default |
Previous Message | Sergei Kornilov | 2019-03-28 08:07:59 | Re: REINDEX CONCURRENTLY 2.0 |