kezhuw commented on code in PR #2006:
URL: https://github.com/apache/zookeeper/pull/2006#discussion_r1228921954
##########
zookeeper-server/src/main/java/org/apache/zookeeper/ZKWatchManager.java:
##########
@@ -225,18 +237,6 @@ void containsWatcher(String path, Watcher watcher,
Watcher.WatcherType watcherTy
synchronized (childWatches) {
containsWatcher = contains(path, watcher, childWatches);
}
-
Review Comment:
Here are my findings:
* `containsWatcher` is package private and used solely by
`ZKWatchManager::removeWatcher`.
* `removeWatcher` enforces more stringent `NoWatcherException` check than
`containsWatcher`.
So, we are in lucking path that partially removing `WatcherType::Data` from
`AddWatchMode::Persistent` is an error even it is permitted by server
side(a.k.a `rc` is 0) and `containsWatcher`.
I pushed a test commit for this. I also pushed tests in
https://github.com/kezhuw/zookeeper/commit/8c35fcecef65c8f5f41379ffd3e1bcf8744f4551
before ZOOKEEPER-4466. All these tests result in `NOWATCHER` when removing
`WatcherType::Data` from `AddWatchMode::Persistent`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
