Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL

Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> writes:
>
>>The assumption I'm making is that if the TCP_* values are present at
>>compile time, then you can make a setsockopt() call and get a sane error
>>code back if there's no support -- rather than a segfault, or having the
>>OS spontaneously do weird things to the connection, or anything like
>>that. Is that a reasonable thing to assume?
>
>
> Well, on a sane OS it's reasonable. I dunno about Windows ;-)
>
> One question to ask is whether we should treat the setsockopt failure
> as fatal or not. It seems to me that aborting the connection could
> reasonably be called an overreaction to a bad parameter setting;
> couldn't we just set the GUC variable to zero and keep going?

There's no real reason why not; currently the code looks like this:

> /* Set default keepalive parameters. This should also catch
> * misconfigurations (non-zero values when socket options aren't
> * supported)
> */
> if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != STATUS_OK)
> return STATUS_ERROR;
>
> if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != STATUS_OK)
> return STATUS_ERROR;
>
> if (pq_setkeepalivescount(tcp_keepalives_count, port) != STATUS_OK)
> return STATUS_ERROR;

We could just log (already done inside pq_*, IIRC) and continue, instead
of erroring out. It's just the way it is because I personally prefer
misconfigurations to break loudly, so you have to fix them ;-)

-O