-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-22716:upgrade zookeeper version to 3.5.5 #394
Conversation
maoling
commented
Jul 19, 2019
- This zookeeper 3.5.5 release is a stable one and recommended for production use, which has no compatibility issues for the native java client api which hbase used.
- Let's listen to the QA report before reviewing.
- more details in the HBASE-22716
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
If we still plan to support 3.4.x zookeeper server, I do not think we should upgrade the zookeeper dependency? @maoling What do you think? Thanks. |
|
Any references? Will the zookeeper team test the compatibility for 3.4.x server when releasing new 3.5.x zookeeper? Thanks. |
And what is the advantage to upgrade to 3.5.x? The 3.4.x will be EOL soon? |
Plz look at this release note. 3.5.x has various performance and stability improvements.
@anmolnar is one of the most qualified person who will share the release planning with you:). |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
They should be compatible as long as you're not using some new API. There was a discussion about the topic on the user list recently: http://markmail.org/thread/gtjod534odnb3w4y
Support 3.5 servers, take advantage of new features, performance, stability improvements. Please check out the release notes.
I think 3.4.x will be EOL later rather than sooner. We're going to have a new major release 3.6.0 some time this year and after that we'll deco the 3.4.x branch in order to avoid supporting 3 major branches at the same time. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
But if we still want to support 3.4 servers, we can not use the new features for 3.5? |
Correct. I can imagine some quick workarounds to resolve this: for instance, using feature switches or new features could be introduced in a new major release only. I believe Curator also has some capabilities to detect ZooKeeper server version and enables internal mechanisms accordingly. In order to support multiple ZooKeeper server versions I suggest to duplicate certain HBase nightly tests and run against both server versions. It's completely up to the Hbase community: if you don't want to upgrade, you can stay on 3.4 and support 3.4 only, but be aware that ZooKeeper project has waken up recently and moving forward. |
💔 -1 overall
This message was automatically generated. |
Looks like my questions from JIRA have been mostly answered. Had these two nits outstanding.
Thanks. |
@anmolnar Just give us a timeline, on when will the zookeeper team announce that 3.4.x is EOL. Then we can move on to 3.5.x on master at least. |
@Apache9 I cannot give you the timeline at the moment, because ZK community hasn't decided yet. That means you'll be good on 3.4 for a long period of time, at least this year for sure. Roughly the plan is:
We're in the middle of (2.), so there's no rush. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great :)
I had a few minor comments, please take a look.
I know this PR is closed for now, but still hoping it will be re-opened soon. (currently I am using this code locally to build HBase with Zookeeper 3.5.5)
@@ -61,7 +62,7 @@ public HACK_UNTIL_ZOOKEEPER_1897_ZooKeeperMain(String[] args) | |||
* @throws IOException in case of a network failure | |||
* @throws InterruptedException if the ZooKeeper client closes | |||
*/ | |||
void runCmdLine() throws KeeperException, IOException, InterruptedException { | |||
void runCmdLine() throws CliException, IOException, InterruptedException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also remove the KeeperException from the Javadoc
@@ -26,6 +26,7 @@ | |||
import org.apache.yetus.audience.InterfaceAudience; | |||
import org.apache.zookeeper.KeeperException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also remove the import for KeeperException
@@ -1471,7 +1471,7 @@ | |||
<protobuf.plugin.version>0.5.0</protobuf.plugin.version> | |||
<thrift.path>thrift</thrift.path> | |||
<thrift.version>0.12.0</thrift.version> | |||
<zookeeper.version>3.4.10</zookeeper.version> | |||
<zookeeper.version>3.5.5</zookeeper.version> | |||
<!-- What ZooKeeper 3.4.x depends on and nothing more --> | |||
<jline.version>0.9.94</jline.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also update the jline version? I think it was added because of zookeeper, see https://issues.apache.org/jira/browse/HBASE-20108
zookeeper 3.5.5 is using jline 2.11
however, if we increment the JLine version, then the assembly:single maven target will fail, as the license part in JLine pom changed a bit (they named it 'The BSD License' instead of 'BSD License'). To make it work, we need to add a new supplement to supplemental-models.xml
like:
<supplement>
<project>
<groupId>jline</groupId>
<artifactId>jline</artifactId>
<name>JLine</name>
<version>2.11</version>
<licenses>
<license>
<name>BSD License</name>
<url>http://www.opensource.org/licenses/bsd-license.php</url>
<distribution>repo</distribution>
</license>
</licenses>
</project>
</supplement>
@@ -1471,7 +1471,7 @@ | |||
<protobuf.plugin.version>0.5.0</protobuf.plugin.version> | |||
<thrift.path>thrift</thrift.path> | |||
<thrift.version>0.12.0</thrift.version> | |||
<zookeeper.version>3.4.10</zookeeper.version> | |||
<zookeeper.version>3.5.5</zookeeper.version> | |||
<!-- What ZooKeeper 3.4.x depends on and nothing more --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment was also added by https://issues.apache.org/jira/browse/HBASE-20108
we can update it to "What ZooKeeper 3.5.5 depends on...", but I think it is a bit hard to decode this comment and I would change it to something like this: "providing JLine on the classpath for zookeeper (see HBASE-20108)"
FYI: Hadoop trunk already uses Curator 4.2 and ZooKeeper 3.5.6 |