ctubbsii commented on code in PR #5256:
URL: https://github.com/apache/accumulo/pull/5256#discussion_r1920338043
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooCache.java:
##########
@@ -152,53 +189,53 @@ public void process(WatchedEvent event) {
break;
}
- externalWatcher.ifPresent(w -> w.accept(event));
+ externalWatchers.forEach(ew -> ew.accept(event));
}
}
- /**
- * Creates a new cache without an external watcher.
- *
- * @param zk the ZooKeeper instance
- * @throws NullPointerException if zk is {@code null}
- */
- public ZooCache(ZooSession zk) {
- this(zk, Optional.empty(), Duration.ofMinutes(3));
+ public ZooCache(ZooSession zk, String root, ZooCacheWatcher... watchers) {
+ this.zk = requireNonNull(zk);
+ for (ZooCacheWatcher zcw : watchers) {
+ externalWatchers.add(zcw);
+ }
+ setupWatchers(requireNonNull(root));
Review Comment:
That's a good question. I think the persistent recursive watcher would need
to be re-added to the new session each time it re-establishes a session.
Otherwise, after establishing a new ZooKeeper session, the watcher will never
be fired. Previously, it was fine, because we'd just invalidate the cache, and
then the next request for getData or getChildren would re-establish a watcher
on the newly established session. That's no longer the case, since we now pass
`null` for the getData and getChildren calls, relying on this watcher instead.
In order for these persistent watchers to survive a session reconnection, I
think we'd need to register them with ZooSession, track them there, and re-add
them every time ZooSession re-establishes a new ZooKeeper session. That can
probably be done, but I'm starting to wonder if any of this is worth it.
--
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]
