RFR:8194298 Add support for per Socket configuration of TCP keepalive

[Alan Bateman]

On 23/04/2018 13:05, vyom tewari wrote:
> Hi,
>
> Please find the latest 
> webrev(http://cr.openjdk.java.net/~vtewari/8194298/webrev0.1/index.html). 
> I incorporated  most of the review comments.
This looks much better but I think the second paragraph of the spec of 
each option needs to be inverted so that it states clearly what the 
options does before it gets into the background of SO_KEEPALIVE. For 
example, TCP_KEEPALIVE should say that it sets the idle period for keep 
alive before it explains SO_KEEPALIVE. The currently wording also begs 
questions as to whether the socket option means anything when 
SO_KEEPALIVE is not enabled. Also it probably should say something about 
whether it can be changed at any time or not.

The implementation looks much better but the filtering by socket option 
name is still a bit fragile. It might be better for 
ExtendedSocketOptions to mainain 3 sets of options, one for SOCK_UNSPEC, 
one for SOCK_STREAM, the other for SOCK_DGRAM. A map would work too. The 
register method can specify socket type and it's very obvious which 
sockets the option is intended to be used for.

All usages are now ExtendedSocketOptions.getInstance().options(...). 
Have you considered a static options(...) method to reduce the typing at 
each of the usage sites?

-Alan