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


##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java:
##########
@@ -59,77 +62,72 @@ public interface ZooCacheWatcher extends 
Consumer<WatchedEvent> {}
 
   private static final Logger log = LoggerFactory.getLogger(ZooCache.class);
 
-  private final ZCacheWatcher watcher = new ZCacheWatcher();
-  private final Optional<ZooCacheWatcher> externalWatcher;
+  protected volatile NavigableSet<String> watchedPaths =
+      Collections.unmodifiableNavigableSet(new TreeSet<>());
+  // visible for tests
+  protected final ZCacheWatcher watcher = new ZCacheWatcher();
+  private final List<ZooCacheWatcher> externalWatchers =
+      Collections.synchronizedList(new ArrayList<>());
 
   private static final AtomicLong nextCacheId = new AtomicLong(0);
   private final String cacheId = "ZC" + nextCacheId.incrementAndGet();
 
-  // The concurrent map returned by Caffiene will only allow one thread to run 
at a time for a given
-  // key and ZooCache relies on that. Not all concurrent map implementations 
have this behavior for
-  // their compute functions.
+  public static final Duration CACHE_DURATION = Duration.ofMinutes(30);
+
+  private final Cache<String,ZcNode> cache;
+
   private final ConcurrentMap<String,ZcNode> nodeCache;
 
   private final ZooSession zk;
 
   private volatile boolean closed = false;
 
-  public static class ZcStat {
-    private long ephemeralOwner;
-    private long mzxid;
+  private final AtomicLong updateCount = new AtomicLong();
 
-    public ZcStat() {}
+  private final AtomicLong zkClientTracker = new AtomicLong();
 
-    private ZcStat(Stat stat) {
-      this.ephemeralOwner = stat.getEphemeralOwner();
-      this.mzxid = stat.getMzxid();
-    }
-
-    public long getEphemeralOwner() {
-      return ephemeralOwner;
-    }
-
-    private void set(ZcStat cachedStat) {
-      this.ephemeralOwner = cachedStat.ephemeralOwner;
-      this.mzxid = cachedStat.mzxid;
-    }
-
-    @VisibleForTesting
-    public void setEphemeralOwner(long ephemeralOwner) {
-      this.ephemeralOwner = ephemeralOwner;
-    }
-
-    public long getMzxid() {
-      return mzxid;
-    }
-  }
-
-  private final AtomicLong updateCount = new AtomicLong(0);
-
-  private class ZCacheWatcher implements Watcher {
+  class ZCacheWatcher implements Watcher {
     @Override
     public void process(WatchedEvent event) {
       if (log.isTraceEnabled()) {
         log.trace("{}: {}", cacheId, event);
       }
 
       switch (event.getType()) {
-        case NodeDataChanged:
         case NodeChildrenChanged:
-        case NodeCreated:
-        case NodeDeleted:
+          // According to documentation we should not receive this event.
+          // According to https://issues.apache.org/jira/browse/ZOOKEEPER-4475 
we
+          // may receive this event (Fixed in 3.9.0)
+          break;
         case ChildWatchRemoved:
         case DataWatchRemoved:
-          remove(event.getPath());
+          // We don't need to do anything with the cache on these events.
+          break;
+        case NodeDataChanged:
+          log.trace("{} node data changed; clearing {}", cacheId, 
event.getPath());
+          clear(path -> path.equals(event.getPath()));
+          break;
+        case NodeCreated:
+        case NodeDeleted:
+          // With the Watcher being set at a higher level we need to remove
+          // the parent of the affected node and all of its children from the 
cache
+          // so that the parent and children node can be re-cached. If we only 
remove the
+          // affected node, then the cached children in the parent could be 
incorrect.
+          int lastSlash = event.getPath().lastIndexOf('/');
+          String parent = lastSlash == 0 ? "/" : event.getPath().substring(0, 
lastSlash);
+          log.trace("{} node created or deleted {}; clearing {}", cacheId, 
event.getPath(), parent);
+          clear((path) -> path.startsWith(parent));
+          break;
+        case PersistentWatchRemoved:
+          log.trace(
+              "{} persistent watch removed {} which is only done in 
ZooSession.addPersistentRecursiveWatchers; ignoring;",
+              cacheId, event.getPath());

Review Comment:
   Agreed. I added this logging in case it does happen for some reason in the 
future.



-- 
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: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to