zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Enrico Olivelli <eolive...@gmail.com>
Subject Re: JAVA 11 build is broken on 3.5
Date Fri, 04 Jan 2019 15:27:00 GMT
Il giorno ven 4 gen 2019 alle ore 14:23 Andor Molnar
<andor@apache.org> ha scritto:
> Hi team / Enrico,
> I’d like to get feedback from the community on the following patch (moving the discussion
from GitHub to here):
> https://issues.apache.org/jira/browse/ZOOKEEPER-3204 <https://issues.apache.org/jira/browse/ZOOKEEPER-3204>
> https://github.com/apache/zookeeper/pull/753 <https://github.com/apache/zookeeper/pull/753>
> In a nutshell: looks like that Netty 3.10 is broken under Java 11: it doesn’t properly
close the underlying socket (probably not closing the registered NIO selectors) and reconfig
tests are unable to re-bind the ports. This problem is similar that we already fixed in NIO
with the following patch:
> https://github.com/apache/zookeeper/commit/c3babb94275ad667dc71c10dcb08a383a3c154c2 <https://github.com/apache/zookeeper/commit/c3babb94275ad667dc71c10dcb08a383a3c154c2>
> The problem doesn’t show up on trunk which has been recently upgraded to Netty 4.
> Repro:
> - Start embedded ZK, stop it and try to restart on the same port, or
> - Start normal ensemble and reconfig to use different (client) port. Then reconfig back
to the original port which should fail. (that’s the scenario which is covered in ReconfigTest)
> I created the above patch (#753) to backport Netty 4 upgrade to 3.5 and it fixes the
problem with Java 11 (it doesn’t cause regression in the pre-commit build either), but Enrico
is having concerns about making such big change before the release.
> I tend to agree, but let’s see what are the options.
> Thoughts:
> - Do we have to fix this? - Yes. Java 11 is LTS and I the bug is critical.
> - Can we fix Netty 3? - Maybe. Let’s say we find the bug in Netty 3, what can we do?
>        a) We cannot workaround from ZooKeeper itself and have to submit a pull request
for Netty. I think it’s quite unlikely that they will accept the change given it’s not
a security bug, but even if they did, only the upgraded version of Netty 3 would work properly
with ZooKeeper. Err.
>        b) We can workaround it from ZooKeeper: that could be option #1, but I have a
strong feeling about it’s not going to be the case.
> - Shall we upgrade to Netty 4? - this is option #2
> Please share your thoughts, maybe you know about an option #3.

Thank you Andor

I have thought more about this problem, and I have checked that Netty
3 is really dead/unmantained (last release in 2016).
If I understand correctly there is no easy workaround (nothing without
hacking Netty 3 internals)

As soon as we will declare 3.5.5 "stable" the world will hopefully
abandon 3.4 and switch to 3.5 + Netty (because of SSL support).
The network stack is very important so it is better to have Netty 4 as
foundation, I am thinking about security issues, we won't make an
"hotfix" release with the switch to Netty 4 because there is a bad bug
in Netty 3.
So better to switch now.

But Facebooks friends, expecially @ivmaykov did a lot of bugfixes
around Netty on master branch, we must be sure that what we are
delivering in 3.5.5 is stable.

We will also have to state clearly in the "release notes" that Netty
version is changed, as this may have a non trivial impact to memory
usage (i.e. Netty 4 uses more Direct memory by default)

So to recap my final opinion: +1 to switch to Netty 4 if we take care
of port all of the fixes around Netty 4 from master branch and we
state the switch clearly in the release notes


> Regards,
> Andor

View raw message