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
