-
Notifications
You must be signed in to change notification settings - Fork 1k
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
PHOENIX-6723 Update client to use Zookeeper 3.5.7 and Curator 4.2.0 #1434
PHOENIX-6723 Update client to use Zookeeper 3.5.7 and Curator 4.2.0 #1434
Conversation
This is relevant for the https://github.com/trinodb/trino project as we want to move to JDK17 (which is the LTS) |
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.
+1 assuming tests pass
Please open a JIRA, and add JIRA-XXXXX to the commit message, @wendigo. |
The ZK version doesn't do anything for ZK on master, we're already using 3.5.7 for both 2.3 and 2.4. (set in the hbase profiles) Please remove the ZK version overrides from the hbase profiles in the pom, as they are not needed with this change. As for Curator, HBase uses 4.2, not 4.3 on branch 2.3, 2.4, and 2.5. Is there a particular reason for using 4.3, or did you just pick the latest 4.3 release ? In case you also want this merged into 5.1, we'd also have to check if HBase 2.1 and 2.2 works with ZK 3.5.7 (IIRC it does) |
@vincentpoon @wendigo - I think this PR is currently waiting for changes that have been requested by @stoty and then I think would be ready for commit. Do you have bandwidth to get back to this in time for the Phoenix 5.2 release (probably in the next month or so?) |
@gjacoby126 I will update this PR soon |
c0cf420
to
4e11281
Compare
@stoty @gjacoby126 PR updated. I tested it with patched Trino version, and now it works correctly on JDK 17 with HBase 2.3.0 |
@wendigo - the changes to the pom look good, but could you please change the commit message to refer to a JIRA number. (And if there isn't a JIRA for this yet, could you please create on on issues.apache.org.) Thanks! |
@gjacoby126 yes, will do, I need to create jira account first. |
@wendigo - if you're creating a JIRA account for the first time, please let us know the username so we can add you to the contributors list. (This will give you the ability to assign the ticket to yourself.) |
Previous Zookeeper version was broken on JDK 14+ due to the incorrect usage of InetSocketAddress.toString. For reference see: https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8232369
4e11281
to
460491c
Compare
@gjacoby126 done, JIRA handle is the same as in GitHub (@wendigo). I've updated the commit message too. |
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.
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.
+1 LGTM.
Thank you.
Previous Zookeeper version was broken on JDK 14+ due to the incorrect usage of
InetSocketAddress.toString. For reference see: https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8232369