[8u] RFR(m): 8194298: Add support for per Socket configuration of TCP keepalive
Severin Gehwolf
sgehwolf at redhat.com
Mon Jun 29 08:54:17 UTC 2020
Hi Vyom!
On Fri, 2020-06-26 at 15:55 +0530, Vyom Tiwari wrote:
> Hi Severin,
>
> Overall code changes looks ok to me, I build & tested at my local REL
> it worked fine for me.
Thanks for the review!
> Below are few minor comments.
>
> 1-> Net.java
> 1.1-> I think you don't need to explicitly convert value to integer and then pass it. You can avoid the local int variable creation as follows.
> ExtendedOptionsImpl.setTcpKeepAliveIntvl(fd, (Integer)value);
> 1.2-> In getSocketOption you don't need to explicitly cast it to Integer.
> return ExtendedOptionsImpl.getTcpKeepAliveIntvl(fd);
> 2-> PlainSocketImpl.java
> 1.1 -> In setOption(SocketOption<T> name, T value) you can avoid the local int variable.
Should all be fixed.
> 3-> Any specific reason for two set of functions "setTcpKeepAliveTime0 and setTcpKeepAliveTime" for all three socket options ?
Not really, other than keeping the backport aligned to the JDK 11u
patch. I've updated it in the latest webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8194298/jdk8/05/webrev/
Thanks,
Severin
> Thanks,
> Vyom
>
> On Fri, Jun 26, 2020 at 1:08 PM Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > Hi,
> >
> > On Thu, 2020-06-25 at 23:55 +0000, Bernd Eckenfels wrote:
> > > This would be a great addition.
> >
> > Thanks.
> >
> > > I do not understand why it does not support the options available for
> > > Windows. Especially given the fact that it actually implements 6
> > > native methods to print "Unsupported".
> > >
> > > But I guess that's less a question to the backport and more to the
> > > general implementation.
> >
> > Yes, indeed. This should be brought up for discussion in JDK head first
> > and implemented there before we can consider a backport.
> >
> > Thanks,
> > Severin
> >
> >
>
>
More information about the jdk8u-dev
mailing list