Update ZooKeeper to 3.6.2 and Curator to 5.1.0 #8590

eolivelli · 2020-11-17T08:52:26Z

Motivation

Upgrade to latest stable ZK version 3.6.2. The new minor versions brings several advantages:

Performance improvements (eg: tuning group commit on txn log)
Prometheus based metrics (so that we can get rid of AspectJ hacky way to instrument ZK)
New features like persistent recursive watches which would greatly simplify the logic to handle metadata cache invalidations.
The possibility of rollback to previous version has also been validated.

Modifications

Update ZooKeeper to 3.6.2
Updated Apache Curator to 5.1.0 (that supports ZK 3.6.x), Curator was implicitly imported by BookKeeper Storage Service

Verifying this change

This change is already covered by existing tests, such as (please describe tests).
Test manually a Pulsar service and Pulsar Standalone
Test ZooKeeper metrics

Problems

Unfortunately we cannot upgrade Curator because there is an error with "pulsar standalone":
Basically we have to upgrade ZooKeeper and Curator on BookKeeper before doing this change here

09:40:36.781 [main] ERROR org.apache.bookkeeper.common.component.AbstractLifecycleComponent - Failed to start Component: zk-storage-container-manager
java.lang.NoSuchMethodError: 'org.apache.curator.framework.listen.ListenerContainer org.apache.curator.framework.recipes.cache.NodeCache.getListenable()'
	at org.apache.bookkeeper.stream.storage.impl.cluster.ZkClusterMetadataStore.watchClusterAssignmentData(ZkClusterMetadataStore.java:169) ~[org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.stream.storage.impl.sc.ZkStorageContainerManager.doStart(ZkStorageContainerManager.java:105) ~[org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.common.component.AbstractLifecycleComponent.start(AbstractLifecycleComponent.java:83) [org.apache.bookkeeper-bookkeeper-common-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.stream.storage.impl.StorageContainerStoreImpl.doStart(StorageContainerStoreImpl.java:97) [org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.common.component.AbstractLifecycleComponent.start(AbstractLifecycleComponent.java:83) [org.apache.bookkeeper-bookkeeper-common-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.stream.server.service.StorageService.doStart(StorageService.java:47) [org.apache.bookkeeper-stream-storage-server-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.common.component.AbstractLifecycleComponent.start(AbstractLifecycleComponent.java:83) [org.apache.bookkeeper-bookkeeper-common-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.common.component.LifecycleComponentStack.lambda$start$4(LifecycleComponentStack.java:144) [org.apache.bookkeeper-bookkeeper-common-4.12.0.jar:4.12.0]
	at com.google.common.collect.ImmutableList.forEach(ImmutableList.java:405) [com.google.guava-guava-30.0-jre.jar:?]
	at org.apache.bookkeeper.common.component.LifecycleComponentStack.start(LifecycleComponentStack.java:144) [org.apache.bookkeeper-bookkeeper-common-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.stream.server.StreamStorageLifecycleComponent.doStart(StreamStorageLifecycleComponent.java:51) [org.apache.bookkeeper-stream-storage-server-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.common.component.AbstractLifecycleComponent.start(AbstractLifecycleComponent.java:83) [org.apache.bookkeeper-bookkeeper-common-4.12.0.jar:4.12.0]
	at org.apache.pulsar.zookeeper.LocalBookkeeperEnsemble.runStreamStorage(LocalBookkeeperEnsemble.java:350) [org.apache.pulsar-pulsar-zookeeper-utils-2.7.0-SNAPSHOT.jar:2.7.0-SNAPSHOT]
	at org.apache.pulsar.zookeeper.LocalBookkeeperEnsemble.startStandalone(LocalBookkeeperEnsemble.java:432) [org.apache.pulsar-pulsar-zookeeper-utils-2.7.0-SNAPSHOT.jar:2.7.0-SNAPSHOT]
	at org.apache.pulsar.PulsarStandalone.start(PulsarStandalone.java:258) [org.apache.pulsar-pulsar-broker-2.7.0-SNAPSHOT.jar:2.7.0-SNAPSHOT]
	at org.apache.pulsar.PulsarStandaloneStarter.main(PulsarStandaloneStarter.java:115) [org.apache.pulsar-pulsar-broker-2.7.0-SNAPSHOT.jar:2.7.0-SNAPSHOT]
09:40:36.782 [main] ERROR org.apache.bookkeeper.common.component.AbstractLifecycleComponent - Failed to start Component: range-service
java.lang.NoSuchMethodError: 'org.apache.curator.framework.listen.ListenerContainer org.apache.curator.framework.recipes.cache.NodeCache.getListenable()'
	at org.apache.bookkeeper.stream.storage.impl.cluster.ZkClusterMetadataStore.watchClusterAssignmentData(ZkClusterMetadataStore.java:169) ~[org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar:4.12.0]
	at org.apache.bookkeeper.stream.storage.impl.sc.ZkStorageContainerManager.doStart(ZkStorageContainerManager.java:105) ~[org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar:4.12.0]

eolivelli · 2020-11-18T08:40:32Z

@sijie @jiazhai
This is the patch on BookKeeper, we could make a 4.12.1 release in the future
apache/bookkeeper#2491

there is no hurry at all

eolivelli · 2021-01-07T09:08:45Z

This patch is waiting for BK 4.12.1 release, a VOTE is pending on BK community

eolivelli · 2021-01-11T13:41:10Z

BK vote completed
now this patch depends on the BK upgrade to 4.12.1
#9019

@codelipenghui PTAL

eolivelli · 2021-01-14T09:08:51Z

@merlimat @codelipenghui this patch is good to go as soon as CI passes

sijie · 2021-01-14T09:44:49Z

managed-ledger/pom.xml

@@ -84,7 +84,16 @@
      <classifier>tests</classifier>
      <scope>test</scope>
    </dependency>
-
+    <dependency>
+       <groupId>io.dropwizard.metrics</groupId>


Can you explain why do we need this? I failed to see this dependency was used here. The same question apply to the other dependency.

because from zk 3.6 onwards the "zookeeper" Maven artifact does not import those dependencies anymore to the dependant projects.
This is because we (ZooKeeper project community) want to say that this artifact is only the "zookeeper java client" and the zookeeper java client does not need such jars.
But we still do not provide a "zookeeper server" jar

In Pulsar we are also running the ZooKeeper server (both for production and for tests) and so we need to explicitly add those two jar to the build, otherwise the ZooKeeper server won't start

It would be good to leave a comment with the reason we're adding these, otherwise down the road it will be difficult to remember the rationale.

@sijie PTAL the description was added on the new deps

eolivelli · 2021-01-15T16:55:06Z

/pulsarbot run-failure-checks

eolivelli · 2021-01-19T07:29:43Z

/pulsarbot run-failure-checks

eolivelli · 2021-01-20T08:29:48Z

/pulsarbot run-failure-checks

eolivelli · 2021-02-09T12:59:17Z

/pulsarbot run-failure-checks

eolivelli · 2021-02-09T17:11:31Z

/pulsarbot run-failure-checks

eolivelli · 2021-02-10T09:39:37Z

/pulsarbot run-failure-checks

eolivelli · 2021-02-10T14:43:24Z

/pulsarbot run-failure-checks

eolivelli · 2021-02-10T16:54:01Z

/pulsarbot run-failure-checks

dlg99 · 2021-02-10T22:23:57Z

distribution/server/src/assemble/LICENSE.bin.txt

-    - io.dropwizard.metrics-metrics-core-3.1.0.jar
-    - io.dropwizard.metrics-metrics-graphite-3.1.0.jar
-    - io.dropwizard.metrics-metrics-jvm-3.1.0.jar
+    - io.dropwizard.metrics-metrics-core-3.2.5.jar


Bookkeeper uses Zookeeper 3.6.2 and dropwizard 3.1.0, I think it makes sense to keep dependency versions in sync to avoid surprises later

@dlg99 I can't remember why I had to make this bump.
Probably it is the same version as in ZK.

I would prefer to upgrade this dep in BK 4.13.

dlg99

LGTM

merlimat · 2021-02-11T04:52:50Z

@sijie PTAL again

This reverts commit 9c39ca0.

…10803) ### Motivation After upgrade zookeeper version to 3.6.2 in #8590 and removed AspectJ based metrics for ZooKeeper in #10533, the zookeeper's prometheus metric has lost if we start zookeeper by `bin/puldar-daemon start zookeeper`. Due to zookeeper 3.6.0+ has add internal prometheus metric provider, so we can turn on by default in pulsar. ### Modification 1. turn on zookeeper prometheus metric provider by default in `conf/zookeeper.conf` and use 8000 as default port sync with old zookeeper metric port 2. add grafana panel for new zookeeper metrics 3. remove old prometheus metric provider in `ZooKeeperStarter` and `ConfigurationStoreStarter`. 4. update reference-metric.md doc

…pache#10803) ### Motivation After upgrade zookeeper version to 3.6.2 in apache#8590 and removed AspectJ based metrics for ZooKeeper in apache#10533, the zookeeper's prometheus metric has lost if we start zookeeper by `bin/puldar-daemon start zookeeper`. Due to zookeeper 3.6.0+ has add internal prometheus metric provider, so we can turn on by default in pulsar. ### Modification 1. turn on zookeeper prometheus metric provider by default in `conf/zookeeper.conf` and use 8000 as default port sync with old zookeeper metric port 2. add grafana panel for new zookeeper metrics 3. remove old prometheus metric provider in `ZooKeeperStarter` and `ConfigurationStoreStarter`. 4. update reference-metric.md doc

…pache#10803) After upgrade zookeeper version to 3.6.2 in apache#8590 and removed AspectJ based metrics for ZooKeeper in apache#10533, the zookeeper's prometheus metric has lost if we start zookeeper by `bin/puldar-daemon start zookeeper`. Due to zookeeper 3.6.0+ has add internal prometheus metric provider, so we can turn on by default in pulsar. 1. turn on zookeeper prometheus metric provider by default in `conf/zookeeper.conf` and use 8000 as default port sync with old zookeeper metric port 2. add grafana panel for new zookeeper metrics 3. remove old prometheus metric provider in `ZooKeeperStarter` and `ConfigurationStoreStarter`. 4. update reference-metric.md doc

…pache#10803) ### Motivation After upgrade zookeeper version to 3.6.2 in apache#8590 and removed AspectJ based metrics for ZooKeeper in apache#10533, the zookeeper's prometheus metric has lost if we start zookeeper by `bin/puldar-daemon start zookeeper`. Due to zookeeper 3.6.0+ has add internal prometheus metric provider, so we can turn on by default in pulsar. ### Modification 1. turn on zookeeper prometheus metric provider by default in `conf/zookeeper.conf` and use 8000 as default port sync with old zookeeper metric port 2. add grafana panel for new zookeeper metrics 3. remove old prometheus metric provider in `ZooKeeperStarter` and `ConfigurationStoreStarter`. 4. update reference-metric.md doc

eolivelli added 2 commits November 17, 2020 09:48

Update ZooKeeper to 3.6.2 and Curator to 5.1.0

add680a

port from apache#8348

47cbe33

eolivelli mentioned this pull request Nov 17, 2020

Upgrade to ZooKeeper 3.6.2 #8348

Closed

eolivelli mentioned this pull request Dec 2, 2020

Upgrade ZooKeeper to 3.6.2 apache/bookkeeper#2503

Merged

codelipenghui assigned eolivelli Jan 11, 2021

codelipenghui added this to the 2.8.0 milestone Jan 11, 2021

codelipenghui requested review from merlimat, sijie, codelipenghui, 315157973, hangc0276 and jiazhai January 11, 2021 15:05

Merge remote-tracking branch 'origin/master' into fix/zk-3.6

b33b9ba

eolivelli marked this pull request as ready for review January 14, 2021 09:08

sijie requested changes Jan 14, 2021

View reviewed changes

eolivelli added 5 commits January 14, 2021 11:42

Exclude shaded ZooKeeper from spotbugs

60835b1

fix spotbugs

e30bf32

fix licenses and tests

d01491f

fix more tests

fe2f2db

More test deps

9e62f7c

Merge remote-tracking branch 'origin/master' into fix/zk-3.6

5440699

Upgrade Curator in Flume connector

d887b9e

merlimat approved these changes Jan 19, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into fix/zk-3.6

822b1dc

eolivelli force-pushed the fix/zk-3.6 branch from 041419b to 822b1dc Compare February 5, 2021 14:58

Merge branch 'master' into fix/zk-3.6

8b2c460

eolivelli mentioned this pull request Feb 8, 2021

CI is not running tests #9526

Closed

Merge branch 'master' into fix/zk-3.6

957ea1e

Fix license check

10487e0

eolivelli added 2 commits February 10, 2021 08:31

Merge branch 'master' into fix/zk-3.6

379dac7

revert typo

a300cac

dlg99 mentioned this pull request Feb 10, 2021

Bookkeper's and pulsar's dependencies are out of sync #9558

Closed

sijie mentioned this pull request Feb 10, 2021

ISSUE-9558: Bookkeper's and pulsar's dependencies are out of sync streamnative/pulsar-archived#2152

Open

dlg99 reviewed Feb 10, 2021

View reviewed changes

dlg99 approved these changes Feb 10, 2021

View reviewed changes

sijie approved these changes Feb 11, 2021

View reviewed changes

sijie merged commit 9c39ca0 into apache:master Feb 11, 2021

eolivelli deleted the fix/zk-3.6 branch February 11, 2021 07:24

eolivelli mentioned this pull request Feb 11, 2021

Upgrade dropwizard metrics to 3.2.5 apache/bookkeeper#2599

Closed

sijie mentioned this pull request Feb 11, 2021

ISSUE-2599: Upgrade dropwizard metrics to 3.2.5 streamnative/bookkeeper-achieved#330

Closed

lhotari added a commit to lhotari/pulsar that referenced this pull request Feb 11, 2021

Revert "Update ZooKeeper to 3.6.2 and Curator to 5.1.0 (apache#8590)"

e8e5c2a

This reverts commit 9c39ca0.

hangc0276 mentioned this pull request Jun 3, 2021

use zookeeper prometheus metric provider to export zookeeper metrics #10803

Merged

tisonkun mentioned this pull request Sep 10, 2022

[improve][test] remove WhiteBox on MockZooKeeper #17579

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ZooKeeper to 3.6.2 and Curator to 5.1.0 #8590

Update ZooKeeper to 3.6.2 and Curator to 5.1.0 #8590

eolivelli commented Nov 17, 2020

eolivelli commented Nov 18, 2020

eolivelli commented Jan 7, 2021

eolivelli commented Jan 11, 2021

eolivelli commented Jan 14, 2021

sijie Jan 14, 2021

eolivelli Jan 14, 2021

merlimat Jan 19, 2021

merlimat Feb 2, 2021

eolivelli commented Jan 15, 2021

eolivelli commented Jan 19, 2021

eolivelli commented Jan 20, 2021

eolivelli commented Feb 9, 2021

eolivelli commented Feb 9, 2021

eolivelli commented Feb 10, 2021

eolivelli commented Feb 10, 2021

eolivelli commented Feb 10, 2021

dlg99 Feb 10, 2021

eolivelli Feb 11, 2021

dlg99 left a comment

merlimat commented Feb 11, 2021

Update ZooKeeper to 3.6.2 and Curator to 5.1.0 #8590

Update ZooKeeper to 3.6.2 and Curator to 5.1.0 #8590

Conversation

eolivelli commented Nov 17, 2020

Motivation

Modifications

Verifying this change

Problems

eolivelli commented Nov 18, 2020

eolivelli commented Jan 7, 2021

eolivelli commented Jan 11, 2021

eolivelli commented Jan 14, 2021

sijie Jan 14, 2021

Choose a reason for hiding this comment

eolivelli Jan 14, 2021

Choose a reason for hiding this comment

merlimat Jan 19, 2021

Choose a reason for hiding this comment

merlimat Feb 2, 2021

Choose a reason for hiding this comment

eolivelli commented Jan 15, 2021

eolivelli commented Jan 19, 2021

eolivelli commented Jan 20, 2021

eolivelli commented Feb 9, 2021

eolivelli commented Feb 9, 2021

eolivelli commented Feb 10, 2021

eolivelli commented Feb 10, 2021

eolivelli commented Feb 10, 2021

dlg99 Feb 10, 2021

Choose a reason for hiding this comment

eolivelli Feb 11, 2021

Choose a reason for hiding this comment

dlg99 left a comment

Choose a reason for hiding this comment

merlimat commented Feb 11, 2021