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

remove ZooKeeper 3.4 support + pass tests with Java 15 #11073

Merged
merged 8 commits into from
May 25, 2021

Conversation

xvrl
Copy link
Member

@xvrl xvrl commented Apr 6, 2021

With this change, Druid will only support ZooKeeper 3.5.x and later.

In order to support Java 15 we need to switch to ZK 3.5.x client libraries and drop support for ZK 3.4.x
(see #10780 for the detailed reasons)

  • remove ZooKeeper 3.4.x compatibility
  • exclude additional ZK 3.5.x netty dependencies to ensure we use our version
  • keep ZooKeeper version used for integration tests in sync with client library version
  • remove the need to specify ZK version at runtime for docker
  • add support to run integration tests with JDK 15
  • build and run unit tests with Java 15 in travis

@xvrl xvrl changed the title Make unit tests pass with Java 15, remove ZooKeeper 3.4 support Make tests pass with Java 15, remove ZooKeeper 3.4 support Apr 6, 2021
@xvrl xvrl marked this pull request as ready for review April 6, 2021 23:43
@xvrl
Copy link
Member Author

xvrl commented Apr 6, 2021

for some reason travis-ci is not picking up this PR 🤔

@xvrl xvrl closed this Apr 6, 2021
@xvrl xvrl reopened this Apr 6, 2021
@xvrl xvrl marked this pull request as draft April 7, 2021 21:07
@xvrl xvrl marked this pull request as ready for review April 7, 2021 21:07
@xvrl xvrl closed this Apr 7, 2021
@xvrl xvrl changed the title Make tests pass with Java 15, remove ZooKeeper 3.4 support remove ZooKeeper 3.4 support + pass tests with Java 15 Apr 7, 2021
@xvrl
Copy link
Member Author

xvrl commented Apr 7, 2021

reopening in the attempt to trigger a travis-ci build

Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concept looks good to me — we've already discussed that in #10780.

The code changes look good to me too.

A couple of questions:

  1. We'll need to have something about this prominently in the release notes, but in addition, I think it'd be good to include it in the docs too. Can you update the docs somewhere to talk about the minimum ZK server version that we support, and the fact that it's been revved recently? Maybe in cluster.md and zookeeper.md.

  2. Should we update Curator too?

@xvrl
Copy link
Member Author

xvrl commented May 1, 2021

Can you update the docs somewhere to talk about the minimum ZK server version that we support, and the fact that it's been revved recently?

Agree, I can add something to the docs to that effect.

Should we update Curator too?

There are a couple more breaking changes in curator 5.0, so we can deal with that separately. Additionally, it lets someone still run against ZK 3.4.x by replacing the client libraries in the classpath if needed. We can probably move to 5.x in Druid 0.23

@xvrl
Copy link
Member Author

xvrl commented May 21, 2021

@gianm added the docs, and rebased on the latest master to fix conflicts

@santosh-d3vpl3x
Copy link
Contributor

Is the milestone for this upgrade 0.21.x or 0.22.x?

@gianm
Copy link
Contributor

gianm commented May 25, 2021

There are a couple more breaking changes in curator 5.0, so we can deal with that separately. Additionally, it lets someone still run against ZK 3.4.x by replacing the client libraries in the classpath if needed. We can probably move to 5.x in Druid 0.23

That's a good point, it'd be nice to retain the ability for people to drop in ZK 3.4 clients for some time. Maybe even a bit beyond Druid 0.23, if the new Curator doesn't offer much exciting stuff.

@xvrl xvrl merged commit b517c33 into apache:master May 25, 2021
@xvrl xvrl deleted the jdk15-zk3.5-unit branch May 25, 2021 19:49
@clintropolis clintropolis added this to the 0.22.0 milestone Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants