Skip to content

CURATOR-596: Upgrade ZooKeeper to 3.7.1 - #426

Merged
tisonkun merged 3 commits into
apache:masterfrom
kezhuw:CURATOR-596-upgrade-zookeeper
Jul 14, 2022
Merged

CURATOR-596: Upgrade ZooKeeper to 3.7.1#426
tisonkun merged 3 commits into
apache:masterfrom
kezhuw:CURATOR-596-upgrade-zookeeper

Conversation

@kezhuw

@kezhuw kezhuw commented Jul 3, 2022

Copy link
Copy Markdown
Member

TestReadOnly fails due to ZOOKEEPER-3863(apache/zookeeper#1380).

ZOOKEEPER-3863 uses QuorumPeerConfig.localSessionsEnabled to toggle
local sessions supports in ReadOnlyZooKeeperServer.

This pr populates QuorumPeerConfig.localSessionsEnabled and
QuorumPeerConfig.localSessionsUpgradingEnabled from system property
readonlymode.enabled for simplicity.

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally looks good. Perhaps we should add a curator-test-zk36 module for running tests with zookeeper 3.6.x to prevent regression? You may take a look at how curator-test-zk35 does now. I'm happy to review this patch :)

cc @kezhuw

`TestReadOnly` fails due to ZOOKEEPER-3863(apache/zookeeper#1380).

ZOOKEEPER-3863 uses `QuorumPeerConfig.localSessionsEnabled` to toggle
local sessions supports in `ReadOnlyZooKeeperServer`.

This pr populates `QuorumPeerConfig.localSessionsEnabled` and
`QuorumPeerConfig.localSessionsUpgradingEnabled` from system property
`readonlymode.enabled` for simplicity.

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@kezhuw
kezhuw force-pushed the CURATOR-596-upgrade-zookeeper branch from e227575 to b83a475 Compare July 13, 2022 15:39
@kezhuw

kezhuw commented Jul 13, 2022

Copy link
Copy Markdown
Member Author

@tisonkun I have rebased this pr to master for 5.4.0-SNAPSHOT and push a fixup commit for module curator-test-zk36. Could you please take a look and trigger ci workflow ?

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun

Copy link
Copy Markdown
Member

@kezhuw Thanks for your contribution!

Merging if CI gives green.

@tisonkun

Copy link
Copy Markdown
Member

Merging...

@kezhuw thank you very much!

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.

3 participants