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

DRILL-8514: Bump zookeeper to 3.9.3 #2967

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

letian-jiang
Copy link

Signed-off-by: Letian Jiang <letian.jiang@outlook.com>
@pjfanning
Copy link
Contributor

Upgrading zookeeper is not that easy. The CI build has already failed with an issue around protobuf.
This is a recent attempt to upgrade - #2893
That attempt had to raise the limit on the jar sizes and still hit test issues with HBase testing. It may not be feasible to upgrade Zookeeper until HBase and maybe other dependencies of Drill have uptaken newer versions of Zookeeper themselves.

@letian-jiang
Copy link
Author

Upgrading zookeeper is not that easy. The CI build has already failed with an issue around protobuf. This is a recent attempt to upgrade - #2893 That attempt had to raise the limit on the jar sizes and still hit test issues with HBase testing. It may not be feasible to upgrade Zookeeper until HBase and maybe other dependencies of Drill have uptaken newer versions of Zookeeper themselves.

Ok. Thanks for the information.

@cgivre cgivre reopened this Dec 26, 2024
@cgivre
Copy link
Contributor

cgivre commented Dec 26, 2024

@letian-jiang Why don't you try also updating the HBase client? Drill is currently running version 2.5 and there is a newer version available on MavenCentral.

@pjfanning
Copy link
Contributor

I created #2968 - maybe we can hold off with zookeeper upgrade until we know if that can be merged ok

@cgivre
Copy link
Contributor

cgivre commented Jan 2, 2025

@pjfanning @letian-jiang All the other dependency libraries have been updated. Please rebase on current master and let's see if we can get this working.

@letian-jiang
Copy link
Author

@cgivre Done. Could you rerun the CI?

@pjfanning
Copy link
Contributor

The hadoop-2 build failure was because this setting is too small

<jdbc-all-jar.maxsize>55000000</jdbc-all-jar.maxsize>

Could you change it to this?

<jdbc-all-jar.maxsize>56000000</jdbc-all-jar.maxsize>

Signed-off-by: Letian Jiang <letian.jiang@outlook.com>
@cgivre cgivre added the backport-to-stable This bug fix is applicable to the latest stable release and should be considered for inclusion there label Jan 2, 2025
@letian-jiang
Copy link
Author

Error: Errors:
Error: PhoenixTestSuite.initPhoenix:54 » Runtime Failed construction of Master: class org.apache.hadoop.hbase.master.HMasterKeeperErrorCode = ConnectionLoss for /hbase
Error: SecuredPhoenixTestSuite.initPhoenix:55 » Runtime Failed construction of Master: class org.apache.hadoop.hbase.master.HMasterKeeperErrorCode = ConnectionLoss for /hbase

@letian-jiang
Copy link
Author

@pjfanning @cgivre Is it ok to simply rerun the CI again? This maybe an accidental error.

@pjfanning
Copy link
Contributor

The test issues seem related to the zookeeper upgrade.

@pjfanning
Copy link
Contributor

I think we should delay this and instead concentrate on upgrading the version of Phoenix that we use. The newer version of Phoenix seems to use newer HBase and Zookeeper versions.

@pjfanning
Copy link
Contributor

I don't know much about Phoenix and upgrading its version is not straightforward - #2972

Another option here is to look at shading the zookeeper classes and including those in one of the Drill jars so that we can upgrade the zookeeper version used by the Drill code but not affect the version used by Phoenix plugin.

@cgivre
Copy link
Contributor

cgivre commented Jan 6, 2025

I think the shading option would work. If someone is using Phoenix and runs into issues we can revisit.

@letian-jiang
Copy link
Author

Thanks for the advice. I will try this in the comming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-stable This bug fix is applicable to the latest stable release and should be considered for inclusion there code-cleanup dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants