dlmarion commented on code in PR #5143:
URL: https://github.com/apache/accumulo/pull/5143#discussion_r1873999370


##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooCache.java:
##########
@@ -207,8 +172,23 @@ public void process(WatchedEvent event) {
    */
   public ZooCache(ZooReader reader, Watcher watcher) {
     this.zReader = reader;
-    nodeCache = new ConcurrentHashMap<>();
     this.externalWatcher = watcher;
+    RemovalListener<String,ZcNode> removalListerner = (path, zcNode, reason) 
-> {
+      try {
+        log.trace("{} removing watches for {} because {}", cacheId, path, 
reason);
+        reader.getZooKeeper().removeWatches(path, watcher, 
Watcher.WatcherType.Any, false);
+      } catch (InterruptedException | KeeperException | RuntimeException e) {
+        log.warn("Failed to remove watches on path {} in zookeeper", path, e);
+      }
+    };
+    // Must register the removal listener using evictionListener inorder for 
removal to be mutually
+    // exclusive with any other operations on the same path. This is important 
for watcher
+    // consistency, concurrently adding and
+    // removing watches for the same path would leave zoocache in a really bad 
state. The cache
+    // builder has another way to register a removal listener that is not 
mutually exclusive.
+    Cache<String,ZcNode> cache = Caffeine.newBuilder().expireAfterAccess(3, 
TimeUnit.MINUTES)

Review Comment:
   There is a `Caches` class that we are using to provide a common way to 
create Caffeine caches.



-- 
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]

Reply via email to