Skip to content
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

STORM-2706: Upgrade to Curator 4.0.0 #2378

Merged
merged 1 commit into from
Oct 23, 2017
Merged

STORM-2706: Upgrade to Curator 4.0.0 #2378

merged 1 commit into from
Oct 23, 2017

Conversation

srdo
Copy link
Contributor

@srdo srdo commented Oct 18, 2017

https://issues.apache.org/jira/browse/STORM-2706

I've tested this with storm-starter running on a distributed mode single-node cluster. I attempted to use storm-perf to see if there was a performance difference, but metrics seem broken on Windows. Might try it out on a Linux box.

I'm hoping people will try this out so we can decide if we want to upgrade.

@HeartSaVioR
Copy link
Contributor

According to https://curator.apache.org/zk-compatibility.html, Curator 4.0.x supports both ZK versions, but we still need to keep an eye of ZK version. If we want to support Zookeeper 3.4.x, we need to exclude Zookeeper dependency from curator-recipes and add Zookeeper dependency manually.

Does the patch work with Zookeeper 3.4.x? We still need to consider supporting Zookeeper 3.4.x first, cause Zookeeper 3.5.x is still announced as 'beta' and most of our end users don't want to explore the beta in production.

@srdo
Copy link
Contributor Author

srdo commented Oct 19, 2017

@HeartSaVioR The exclusion isn't necessary because we set Zookeeper to version 3.4.6 in the dependencyManagement section of the root pom. It ensures that if any child poms depend on Zookeeper without specifying a version they'll get 3.4.6, and if Zookeeper is included transitively it'll also be version 3.4.6.

Excluding Zookeeper from the Curator dependencies is a much more fiddly way of accomplishing the same thing. We're not upgrading Zookeeper with this patch.

@revans2
Copy link
Contributor

revans2 commented Oct 20, 2017

The simplest way to know for sure what you are going to get is to run mvn dependency:tree

$ mvn dependency:tree | grep zookeeper | sort -u
[INFO] +- org.apache.zookeeper:zookeeper:jar:3.4.6:compile
[INFO] |     +- org.apache.zookeeper:zookeeper:jar:3.4.6:compile
[INFO] |  +- org.apache.zookeeper:zookeeper:jar:3.4.6:compile
[INFO] |  +- org.apache.zookeeper:zookeeper:jar:3.4.6:provided
[INFO] |  +- org.apache.zookeeper:zookeeper:jar:3.4.6:test
[INFO] |  \- org.apache.zookeeper:zookeeper:jar:3.4.6:compile
[INFO] |  |  +- org.apache.zookeeper:zookeeper:jar:3.4.6:provided

I did a diff of the dependency trees both before and after this patch. They are identical except for the version of curator has changed.

I am +1 for this change

@HeartSaVioR
Copy link
Contributor

@revans2 Thanks for making it clear. I'm also +1 then.

@asfgit asfgit merged commit 7f6d706 into apache:master Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants