Make ZooCache close() and clear() more idempotent

Author: DomGarguilo

core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java

@@ -31,6 +31,7 @@
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
@@ -79,7 +80,7 @@ public interface ZooCacheWatcher extends Consumer<WatchedEvent> {}
 
   private final ZooSession zk;
 
-  private volatile boolean closed = false;
+  private final AtomicBoolean closed = new AtomicBoolean(false);
 
   private final AtomicLong updateCount = new AtomicLong();
 
@@ -349,7 +350,7 @@ private ZcInterruptedException(InterruptedException e) {
    * @return children list, or null if node has no children or does not exist
    */
   public List<String> getChildren(final String zPath) {
-    Preconditions.checkState(!closed);
+    Preconditions.checkState(!closed.get(), "Operation not allowed: ZooCache is already closed.");
     ensureWatched(zPath);
     ZooRunnable<List<String>> zr = new ZooRunnable<>() {
 
@@ -409,7 +410,7 @@ public byte[] get(final String zPath) {
    * @return path data, or null if non-existent
    */
   public byte[] get(final String zPath, final ZcStat status) {
-    Preconditions.checkState(!closed);
+    Preconditions.checkState(!closed.get(), "Operation not allowed: ZooCache is already closed.");
     ensureWatched(zPath);
     ZooRunnable<byte[]> zr = new ZooRunnable<>() {
 
@@ -474,7 +475,7 @@ public byte[] run() throws KeeperException, InterruptedException {
    * @param cachedStat cached statistic, that is or will be cached
    */
   protected void copyStats(ZcStat userStat, ZcStat cachedStat) {
-    Preconditions.checkState(!closed);
+    Preconditions.checkState(!closed.get(), "Operation not allowed: ZooCache is already closed.");
     if (userStat != null && cachedStat != null) {
       userStat.set(cachedStat);
     }
@@ -484,23 +485,30 @@ protected void copyStats(ZcStat userStat, ZcStat cachedStat) {
    * Clears this cache.
    */
   protected void clear() {
-    Preconditions.checkState(!closed);
+    if (closed.get()) {
+      log.trace("clear() called on closed ZooCache {}. Returning.", cacheId);
+      return;
+    }
     nodeCache.clear();
     updateCount.incrementAndGet();
     log.trace("{} cleared all from cache", cacheId);
   }
 
   public void close() {
-    clear();
-    closed = true;
+    if (!closed.get()) {
+      clear();
+      closed.set(true);
+    } else {
+      log.trace("close() called on already closed ZooCache {}", cacheId);
+    }
   }
 
   /**
    * Returns a monotonically increasing count of the number of time the cache was updated. If the
    * count is the same, then it means cache did not change.
    */
   public long getUpdateCount() {
-    Preconditions.checkState(!closed);
+    Preconditions.checkState(!closed.get(), "Operation not allowed: ZooCache is already closed.");
     return updateCount.get();
   }
 
@@ -534,7 +542,7 @@ public boolean childrenCached(String zPath) {
    * Removes all paths in the cache match the predicate.
    */
   public void clear(Predicate<String> pathPredicate) {
-    Preconditions.checkState(!closed);
+    Preconditions.checkState(!closed.get(), "Operation not allowed: ZooCache is already closed.");
     Predicate<String> pathPredicateWrapper = path -> {
       boolean testResult = pathPredicate.test(path);
       if (testResult) {