kezhuw commented on code in PR #1859:
URL: https://github.com/apache/zookeeper/pull/1859#discussion_r1125290082
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/watch/WatchManager.java:
##########
@@ -232,49 +249,49 @@ public synchronized void dumpWatches(PrintWriter pwriter,
boolean byPath) {
@Override
public synchronized boolean containsWatcher(String path, Watcher watcher) {
- WatcherMode watcherMode = watcherModeManager.getWatcherMode(watcher,
path);
- PathParentIterator pathParentIterator = getPathParentIterator(path);
- for (String localPath : pathParentIterator.asIterable()) {
- Set<Watcher> watchers = watchTable.get(localPath);
- if (!pathParentIterator.atParentPath()) {
- if (watchers != null) {
- return true; // at the leaf node, all watcher types
match
- }
- }
- if (watcherMode.isRecursive()) {
- return true;
- }
- }
- return false;
+ Set<Watcher> list = watchTable.get(path);
Review Comment:
> It seems that previously, containsWatcher (path, watcher) will be true if
any parent of the path has a persistent recursive watch.
It is no true though it might intend to do so. Two cases for old code:
* If there is a watch for (watcher, path), the first `return true` will hold.
* If there is no watch for (watcher, path), `watcherMode` will be
`WatcherMode.STANDARD`, no `return true` will hold:
* First `return true` will not hold as it only apply for leaf node where
there is no such watch.
* Second `return true` will not hold as `WatcherMode.STANDARD` is no
recursive.
So my change has same behavior as old code and also same to when it was
introduced. I think this behavior meets what
[`OpCode.checkWatches`](https://issues.apache.org/jira/browse/ZOOKEEPER-1910)
expects which is the [sole client visible call to this
method](https://github.com/apache/zookeeper/blob/de8768807fc7a3fcabc9762b033cf59e695cf14b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java#L507).
Disregard [cheating behaviour of
`ZooKeeper.removeWatches`](https://issues.apache.org/jira/browse/ZOOKEEPER-4625),
removing sub nodes from recursive watching sounds quirk as similar one I saw
in [ZOOKEEPER-4471](https://issues.apache.org/jira/browse/ZOOKEEPER-4471).
--
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]
