-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Use ZK persistent watches #11198
Use ZK persistent watches #11198
Conversation
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.
I like this work.
I believe that this feature is great and it will help a lot in saving resources.
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/ZKMetadataStore.java
Outdated
Show resolved
Hide resolved
@@ -76,6 +79,7 @@ public ZKMetadataStore(String metadataURL, MetadataStoreConfig metadataStoreConf | |||
} | |||
})) | |||
.build(); | |||
zkc.addWatch("/", this::handleWatchEvent, AddWatchMode.PERSISTENT_RECURSIVE); |
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.
Do you use chroot or does this really mean to watch the entire ZK tree?
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.
The Chroot is eventually configured on the ZK connection string. This will have 1 single watch to receive notifications for all the updates related to Pulsar/BK
cb28038
to
97bead7
Compare
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.
LGTM
We should note that with this change Pulsar won't be able to work with a ZooKeeper 3.5 server
or in case the ZooKeeper is configured with zookeeper.watchManagerName= WatchManagerOptimized
http://zookeeper.apache.org/doc/r3.7.0/zookeeperAdmin.html
so people who want to upgrade from an older version of Pulsar (like 2.7.x) that bundled ZooKeeper 3.5 must upgrade the ZooKeeper nodes before upgrading the Brokers
and the same applies to people who are using a dedicated existing ZooKeeper cluster: they will have to upgrade ZooKeeper to 3.6.x before updating Pulsar
I added |
I have created this follow up issue |
I wonder if this change introduced this issue: #11637 . |
@merlimat I got a problem when I tried to upgrade my pulsar cluster from 2.8.0 to 2.9.1, my existing zookeeper server cluster version is 3.5.5. However, the upgraded pulsar cluster failed to start, because of this exception ### org.apache.zookeeper.KeeperException$UnimplementedException, the stacktrace is as followed:
I looked into the source code. the above exception is thrown at zkc.addWatch in ZKMetadataStore. I guess that this method call forces me to upgrade my zookeeper to a more update to date version, maybe 3.6.3. right? I am wondering is there any problem with my cluster metadata when I migrate my data from the old zookeeper ? |
I have encountered the same mistake |
Motivation
Instead of setting watches each time we do getData or exist operation, it's more efficient and less error prone to set a single persistent recursive watch to do the notifications.