accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] keith-turner closed pull request #372: ACCUMULO-4779 Improved performance get by prefix in config
Date Thu, 01 Jan 1970 00:00:00 GMT
keith-turner closed pull request #372: ACCUMULO-4779 Improved performance get by prefix in
config
URL: https://github.com/apache/accumulo/pull/372
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
index b388f77429..9f68d16b00 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
@@ -175,11 +175,6 @@ public static AccumuloConfiguration convertClientConfig(final Configuration
conf
 
     return new AccumuloConfiguration() {
 
-      @Override
-      protected String getArbitrarySystemPropertyImpl(String property) {
-        return config.getString(property, null);
-      }
-
       @Override
       public String get(Property property) {
         final String key = property.getKey();
diff --git a/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
b/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
index a4f6a154f7..244d6f8761 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
@@ -39,11 +39,6 @@ public void put(String k, String v) {
     map.put(k, v);
   }
 
-  @Override
-  protected String getArbitrarySystemPropertyImpl(String property) {
-    return map.get(property);
-  }
-
   @Override
   public String get(Property property) {
     return map.get(property.getKey());
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
index 7e0db72362..3bf54be6d1 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
@@ -16,6 +16,7 @@
  */
 package org.apache.accumulo.core.conf;
 
+import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -23,6 +24,8 @@
 import java.util.Objects;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.client.AccumuloException;
@@ -35,15 +38,28 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableMap;
 
 /**
  * A configuration object.
  */
 public abstract class AccumuloConfiguration implements Iterable<Entry<String,String>>
{
 
+  private static class PrefixProps {
+    final long updateCount;
+    final Map<String,String> props;
+
+    PrefixProps(Map<String,String> props, long updateCount) {
+      this.updateCount = updateCount;
+      this.props = props;
+    }
+  }
+
+  private volatile EnumMap<Property,PrefixProps> cachedPrefixProps = new EnumMap<>(Property.class);
+  private Lock prefixCacheUpdateLock = new ReentrantLock();
+
   /**
    * A filter for properties, based on key.
    *
@@ -109,32 +125,6 @@ public boolean apply(String key) {
 
   private static final Logger log = LoggerFactory.getLogger(AccumuloConfiguration.class);
 
-  protected String getArbitrarySystemPropertyImpl(AccumuloConfiguration parent, String property)
{
-    return parent.getArbitrarySystemPropertyImpl(property);
-  }
-
-  /**
-   * This method is not called with sensitive or per table properties.
-   */
-  protected String getArbitrarySystemPropertyImpl(String property) {
-    throw new UnsupportedOperationException();
-  }
-
-  /**
-   * This method was created because {@link #get(String)} is very slow. However this method
does not properly handle everything that {@link #get(String)} does.
-   * For example it does not properly handle table or sensitive properties.
-   *
-   * <p>
-   * This method has a whitelist of prefixes it handles. To see the whitelist, check the
implementation. When adding to the whitelist, ensure that all
-   * configurations can properly handle.
-   */
-  public String getArbitrarySystemProperty(Property prefix, String property) {
-    Preconditions.checkArgument(prefix == Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
-
-    String key = prefix.getKey() + property;
-    return getArbitrarySystemPropertyImpl(key);
-  }
-
   /**
    * Gets a property value from this configuration.
    *
@@ -193,6 +183,13 @@ private void checkType(Property property, PropertyType type) {
     }
   }
 
+  /**
+   * Each time configurations is changes, this counter should increase.
+   */
+  public long getUpdateCount() {
+    return 0;
+  }
+
   /**
    * Gets all properties under the given prefix in this configuration.
    *
@@ -205,9 +202,41 @@ private void checkType(Property property, PropertyType type) {
   public Map<String,String> getAllPropertiesWithPrefix(Property property) {
     checkType(property, PropertyType.PREFIX);
 
-    Map<String,String> propMap = new HashMap<>();
-    getProperties(propMap, new PrefixFilter(property.getKey()));
-    return propMap;
+    PrefixProps prefixProps = cachedPrefixProps.get(property);
+
+    if (prefixProps == null || prefixProps.updateCount != getUpdateCount()) {
+      prefixCacheUpdateLock.lock();
+      try {
+        // Very important that update count is read before getting properties. Also only
read it once.
+        long updateCount = getUpdateCount();
+        prefixProps = cachedPrefixProps.get(property);
+
+        if (prefixProps == null || prefixProps.updateCount != updateCount) {
+          Map<String,String> propMap = new HashMap<>();
+          // The reason this caching exists is to avoid repeatedly making this expensive
call.
+          getProperties(propMap, new PrefixFilter(property.getKey()));
+          propMap = ImmutableMap.copyOf(propMap);
+
+          // So that locking is not needed when reading from enum map, always create a new
one. Construct and populate map using a local var so its not visible
+          // until ready.
+          EnumMap<Property,PrefixProps> localPrefixes = new EnumMap<>(Property.class);
+
+          // carry forward any other cached prefixes
+          localPrefixes.putAll(cachedPrefixProps);
+
+          // put the updates
+          prefixProps = new PrefixProps(propMap, updateCount);
+          localPrefixes.put(property, prefixProps);
+
+          // make the newly constructed map available
+          cachedPrefixProps = localPrefixes;
+        }
+      } finally {
+        prefixCacheUpdateLock.unlock();
+      }
+    }
+
+    return prefixProps.props;
   }
 
   /**
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java
index df4355760f..a84765a846 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/ConfigurationCopy.java
@@ -27,6 +27,7 @@
  * An {@link AccumuloConfiguration} which holds a flat copy of properties defined in another
configuration
  */
 public class ConfigurationCopy extends AccumuloConfiguration {
+  private long updateCount = 0;
   final Map<String,String> copy = Collections.synchronizedMap(new HashMap<String,String>());
 
   /**
@@ -63,11 +64,6 @@ public String get(Property property) {
     return copy.get(property.getKey());
   }
 
-  @Override
-  protected String getArbitrarySystemPropertyImpl(String property) {
-    return copy.get(property);
-  }
-
   @Override
   public void getProperties(Map<String,String> props, Predicate<String> filter)
{
     for (Entry<String,String> entry : copy.entrySet()) {
@@ -86,7 +82,10 @@ public void getProperties(Map<String,String> props, Predicate<String>
filter) {
    *          property value
    */
   public void set(Property prop, String value) {
-    copy.put(prop.getKey(), value);
+    synchronized (copy) {
+      copy.put(prop.getKey(), value);
+      updateCount++;
+    }
   }
 
   /**
@@ -98,7 +97,16 @@ public void set(Property prop, String value) {
    *          property value
    */
   public void set(String key, String value) {
-    copy.put(key, value);
+    synchronized (copy) {
+      copy.put(key, value);
+      updateCount++;
+    }
   }
 
+  @Override
+  public long getUpdateCount() {
+    synchronized (copy) {
+      return updateCount;
+    }
+  }
 }
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
index 994d960e02..e1ff7e16bb 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/DefaultConfiguration.java
@@ -58,9 +58,4 @@ public void getProperties(Map<String,String> props, Predicate<String>
filter) {
       if (filter.apply(entry.getKey()))
         props.put(entry.getKey(), entry.getValue());
   }
-
-  @Override
-  protected String getArbitrarySystemPropertyImpl(String property) {
-    return null;
-  }
 }
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
index 32b2564a0f..9f047e2ba4 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
@@ -128,15 +128,6 @@ public String get(Property property) {
     return value;
   }
 
-  @Override
-  protected String getArbitrarySystemPropertyImpl(String property) {
-    String val = staticConfigs.get(property);
-    if (val == null) {
-      val = parent.getArbitrarySystemPropertyImpl(property);
-    }
-    return val;
-  }
-
   @Override
   public void getProperties(Map<String,String> props, Predicate<String> filter)
{
     parent.getProperties(props, filter);
diff --git a/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
b/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
index 79db306e63..f724b7b4b0 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoModuleFactory.java
@@ -257,7 +257,7 @@ public static CryptoModuleParameters createParamsObjectFromAccumuloConfiguration
 
   public static CryptoModuleParameters fillParamsObjectFromConfiguration(CryptoModuleParameters
params, AccumuloConfiguration conf) {
     // Get all the options from the configuration
-    Map<String,String> cryptoOpts = conf.getAllPropertiesWithPrefix(Property.CRYPTO_PREFIX);
+    Map<String,String> cryptoOpts = new HashMap<>(conf.getAllPropertiesWithPrefix(Property.CRYPTO_PREFIX));
     cryptoOpts.putAll(conf.getAllPropertiesWithPrefix(Property.INSTANCE_PREFIX));
     cryptoOpts.remove(Property.INSTANCE_SECRET.getKey());
     cryptoOpts.put(Property.CRYPTO_BLOCK_STREAM_SIZE.getKey(), Integer.toString((int) conf.getMemoryInBytes(Property.CRYPTO_BLOCK_STREAM_SIZE)));
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
index f12ba6349f..ba373ab1c5 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/AccumuloConfigurationTest.java
@@ -17,12 +17,26 @@
 package org.apache.accumulo.core.conf;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableMap;
 
 public class AccumuloConfigurationTest {
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   @Test
   public void testGetMemoryInBytes() throws Exception {
     assertEquals(42l, AccumuloConfiguration.getMemoryInBytes("42"));
@@ -147,4 +161,142 @@ public void testGetPortInvalidSyntax() {
     cc.getPort(Property.TSERV_CLIENTPORT);
   }
 
+  private static class TestConfiguration extends AccumuloConfiguration {
+
+    private HashMap<String,String> props = new HashMap<>();
+    private int upCount = 0;
+
+    public void set(String p, String v) {
+      props.put(p, v);
+      upCount++;
+    }
+
+    @Override
+    public long getUpdateCount() {
+      return upCount;
+    }
+
+    @Override
+    public String get(Property property) {
+      return props.get(property.getKey());
+    }
+
+    @Override
+    public void getProperties(Map<String,String> output, Predicate<String> filter)
{
+      for (Entry<String,String> entry : props.entrySet()) {
+        if (filter.apply(entry.getKey())) {
+          output.put(entry.getKey(), entry.getValue());
+        }
+      }
+    }
+
+  }
+
+  @Test
+  public void testMutatePrefixMap() {
+    TestConfiguration tc = new TestConfiguration();
+    tc.set(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a1", "325");
+    tc.set(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a2", "asg34");
+
+    Map<String,String> pm1 = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+
+    Map<String,String> expected1 = new HashMap<>();
+    expected1.put(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a1", "325");
+    expected1.put(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a2", "asg34");
+    assertEquals(expected1, pm1);
+
+    thrown.expect(UnsupportedOperationException.class);
+    pm1.put("k9", "v3");
+  }
+
+  @Test
+  public void testGetByPrefix() {
+    // This test checks that when anything changes that all prefix maps are regenerated.
However when there are not changes the test expects all the exact same
+    // map to always be returned.
+
+    TestConfiguration tc = new TestConfiguration();
+
+    tc.set(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a1", "325");
+    tc.set(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a2", "asg34");
+
+    tc.set(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i1", "class34");
+    tc.set(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i1.opt", "o99");
+
+    Map<String,String> pm1 = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+    Map<String,String> pm2 = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+
+    assertSame(pm1, pm2);
+    Map<String,String> expected1 = new HashMap<>();
+    expected1.put(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a1", "325");
+    expected1.put(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey() + "a2", "asg34");
+    assertEquals(expected1, pm1);
+
+    Map<String,String> pm3 = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    Map<String,String> pm4 = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+
+    assertSame(pm3, pm4);
+    Map<String,String> expected2 = new HashMap<>();
+    expected2.put(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i1", "class34");
+    expected2.put(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i1.opt", "o99");
+    assertEquals(expected2, pm3);
+
+    Map<String,String> pm5 = tc.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
+    Map<String,String> pm6 = tc.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
+    assertSame(pm5, pm6);
+    assertEquals(0, pm5.size());
+
+    // ensure getting one prefix does not cause others to unnecessarily regenerate
+    Map<String,String> pm7 = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+    assertSame(pm1, pm7);
+
+    Map<String,String> pm8 = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    assertSame(pm3, pm8);
+
+    Map<String,String> pm9 = tc.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
+    assertSame(pm5, pm9);
+
+    tc.set(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i2", "class42");
+    tc.set(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i2.opt", "o78234");
+
+    expected2.put(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i2", "class42");
+    expected2.put(Property.TABLE_ITERATOR_SCAN_PREFIX.getKey() + "i2.opt", "o78234");
+
+    Map<String,String> pmA = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    Map<String,String> pmB = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    assertNotSame(pm3, pmA);
+    assertSame(pmA, pmB);
+    assertEquals(expected2, pmA);
+
+    Map<String,String> pmC = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+    Map<String,String> pmD = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+    assertNotSame(pm1, pmC);
+    assertSame(pmC, pmD);
+    assertEquals(expected1, pmC);
+
+    tc.set(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey() + "ctx123", "hdfs://ib/p1");
+
+    Map<String,String> pmE = tc.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
+    Map<String,String> pmF = tc.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
+    assertSame(pmE, pmF);
+    assertNotSame(pm5, pmE);
+    assertEquals(ImmutableMap.of(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey() + "ctx123",
"hdfs://ib/p1"), pmE);
+
+    Map<String,String> pmG = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    Map<String,String> pmH = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    assertNotSame(pmA, pmG);
+    assertSame(pmG, pmH);
+    assertEquals(expected2, pmG);
+
+    Map<String,String> pmI = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+    Map<String,String> pmJ = tc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+    assertNotSame(pmC, pmI);
+    assertSame(pmI, pmJ);
+    assertEquals(expected1, pmI);
+
+    Map<String,String> pmK = tc.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
+    assertSame(pmE, pmK);
+
+    Map<String,String> pmL = tc.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_SCAN_PREFIX);
+    assertSame(pmG, pmL);
+  }
 }
diff --git a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
index 801ee2cef3..3daf20b3f4 100644
--- a/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
+++ b/fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java
@@ -69,33 +69,39 @@
     final Map<String,byte[]> cache;
     final Map<String,Stat> statCache;
     final Map<String,List<String>> childrenCache;
+    final long updateCount;
 
-    ImmutableCacheCopies() {
+    ImmutableCacheCopies(long updateCount) {
+      this.updateCount = updateCount;
       cache = Collections.emptyMap();
       statCache = Collections.emptyMap();
       childrenCache = Collections.emptyMap();
     }
 
-    ImmutableCacheCopies(Map<String,byte[]> cache, Map<String,Stat> statCache,
Map<String,List<String>> childrenCache) {
+    ImmutableCacheCopies(long updateCount, Map<String,byte[]> cache, Map<String,Stat>
statCache, Map<String,List<String>> childrenCache) {
+      this.updateCount = updateCount;
       this.cache = Collections.unmodifiableMap(new HashMap<>(cache));
       this.statCache = Collections.unmodifiableMap(new HashMap<>(statCache));
       this.childrenCache = Collections.unmodifiableMap(new HashMap<>(childrenCache));
     }
 
-    ImmutableCacheCopies(ImmutableCacheCopies prev, Map<String,List<String>>
childrenCache) {
+    ImmutableCacheCopies(long updateCount, ImmutableCacheCopies prev, Map<String,List<String>>
childrenCache) {
+      this.updateCount = updateCount;
       this.cache = prev.cache;
       this.statCache = prev.statCache;
       this.childrenCache = Collections.unmodifiableMap(new HashMap<>(childrenCache));
     }
 
-    ImmutableCacheCopies(Map<String,byte[]> cache, Map<String,Stat> statCache,
ImmutableCacheCopies prev) {
+    ImmutableCacheCopies(long updateCount, Map<String,byte[]> cache, Map<String,Stat>
statCache, ImmutableCacheCopies prev) {
+      this.updateCount = updateCount;
       this.cache = Collections.unmodifiableMap(new HashMap<>(cache));
       this.statCache = Collections.unmodifiableMap(new HashMap<>(statCache));
       this.childrenCache = prev.childrenCache;
     }
   }
 
-  private volatile ImmutableCacheCopies immutableCache = new ImmutableCacheCopies();
+  private volatile ImmutableCacheCopies immutableCache = new ImmutableCacheCopies(0);
+  private long updateCount = 0;
 
   /**
    * Returns a ZooKeeper session. Calls should be made within run of ZooRunnable after caches
are checked. This will be performed at each retry of the run
@@ -285,7 +291,7 @@ public T retry() {
             children = ImmutableList.copyOf(children);
           }
           childrenCache.put(zPath, children);
-          immutableCache = new ImmutableCacheCopies(immutableCache, childrenCache);
+          immutableCache = new ImmutableCacheCopies(++updateCount, immutableCache, childrenCache);
           return children;
         } catch (KeeperException ke) {
           if (ke.code() != Code.NONODE) {
@@ -411,7 +417,7 @@ private void put(String zPath, byte[] data, Stat stat) {
       cache.put(zPath, data);
       statCache.put(zPath, stat);
 
-      immutableCache = new ImmutableCacheCopies(cache, statCache, immutableCache);
+      immutableCache = new ImmutableCacheCopies(++updateCount, cache, statCache, immutableCache);
     } finally {
       cacheWriteLock.unlock();
     }
@@ -424,7 +430,7 @@ private void remove(String zPath) {
       childrenCache.remove(zPath);
       statCache.remove(zPath);
 
-      immutableCache = new ImmutableCacheCopies(cache, statCache, childrenCache);
+      immutableCache = new ImmutableCacheCopies(++updateCount, cache, statCache, childrenCache);
     } finally {
       cacheWriteLock.unlock();
     }
@@ -440,12 +446,19 @@ public void clear() {
       childrenCache.clear();
       statCache.clear();
 
-      immutableCache = new ImmutableCacheCopies();
+      immutableCache = new ImmutableCacheCopies(++updateCount);
     } finally {
       cacheWriteLock.unlock();
     }
   }
 
+  /**
+   * 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() {
+    return immutableCache.updateCount;
+  }
+
   /**
    * Checks if a data value (or lack of one) is cached.
    *
@@ -508,7 +521,7 @@ public void clear(String zPath) {
           i.remove();
       }
 
-      immutableCache = new ImmutableCacheCopies(cache, statCache, childrenCache);
+      immutableCache = new ImmutableCacheCopies(++updateCount, cache, statCache, childrenCache);
     } finally {
       cacheWriteLock.unlock();
     }
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfiguration.java
b/server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfiguration.java
index 1ca083e30b..51783f6e37 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfiguration.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfiguration.java
@@ -167,4 +167,9 @@ public synchronized void invalidateCache() {
     // to see if it happened to be created so we could invalidate its cache
     // but I don't see much benefit coming from that extra check.
   }
+
+  @Override
+  public long getUpdateCount() {
+    return parent.getUpdateCount() + getPropCacheAccessor().getZooCache().getUpdateCount();
+  }
 }
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java
b/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java
index 6c53c5b9f6..8e68d5fb1c 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java
@@ -139,4 +139,9 @@ public synchronized void invalidateCache() {
   public String toString() {
     return this.getClass().getSimpleName();
   }
+
+  @Override
+  public long getUpdateCount() {
+    return parent.getUpdateCount() + getPropCacheAccessor().getZooCache().getUpdateCount();
+  }
 }
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
index cb7bc98f72..79c3806cd9 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java
@@ -78,15 +78,6 @@ private String _get(Property property) {
     return value;
   }
 
-  @Override
-  protected String getArbitrarySystemPropertyImpl(String property) {
-    String val = getRaw(property);
-    if (val == null) {
-      val = getArbitrarySystemPropertyImpl(parent, property);
-    }
-    return val;
-  }
-
   @Override
   public String get(Property property) {
     if (Property.isFixedZooPropertyKey(property)) {
@@ -129,4 +120,9 @@ public void getProperties(Map<String,String> props, Predicate<String>
filter) {
       }
     }
   }
+
+  @Override
+  public long getUpdateCount() {
+    return parent.getUpdateCount() + propCache.getUpdateCount();
+  }
 }
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
index 0cfbff800d..ce62b93bcc 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java
@@ -30,6 +30,8 @@
 import org.apache.accumulo.server.fs.VolumeManager;
 import org.apache.accumulo.server.fs.VolumeManagerImpl;
 import org.apache.hadoop.fs.Path;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
 
 /**
  * A factory for {@link ZooConfiguration} objects.
@@ -69,10 +71,17 @@ ZooConfiguration getInstance(Instance inst, ZooCacheFactory zcf, AccumuloConfigu
       config = instances.get(instanceId);
       if (config == null) {
         ZooCache propCache;
+
+        // The purpose of this watcher is a hack. It forces the creation on a new zoocache
instead of using a shared one. This was done so that the zoocache
+        // would update less, causing the configuration update count to changes less.
+        Watcher watcher = new Watcher() {
+          @Override
+          public void process(WatchedEvent arg0) {}
+        };
         if (inst == null) {
-          propCache = zcf.getZooCache(parent.get(Property.INSTANCE_ZK_HOST), (int) parent.getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT));
+          propCache = zcf.getZooCache(parent.get(Property.INSTANCE_ZK_HOST), (int) parent.getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT),
watcher);
         } else {
-          propCache = zcf.getZooCache(inst.getZooKeepers(), inst.getZooKeepersSessionTimeOut());
+          propCache = zcf.getZooCache(inst.getZooKeepers(), inst.getZooKeepersSessionTimeOut(),
watcher);
         }
         config = new ZooConfiguration(instanceId, propCache, parent);
         instances.put(instanceId, config);
diff --git a/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
b/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
index b0691630e5..4c0e842ada 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/conf/ZooConfigurationFactoryTest.java
@@ -17,8 +17,10 @@
 package org.apache.accumulo.server.conf;
 
 import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.eq;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.isA;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
 import static org.junit.Assert.assertNotNull;
@@ -28,6 +30,7 @@
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
 import org.apache.accumulo.fate.zookeeper.ZooCache;
 import org.apache.accumulo.fate.zookeeper.ZooCacheFactory;
+import org.apache.zookeeper.Watcher;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -54,7 +57,7 @@ public void testGetInstance() {
     expect(instance.getZooKeepers()).andReturn("localhost");
     expect(instance.getZooKeepersSessionTimeOut()).andReturn(120000);
     replay(instance);
-    expect(zcf.getZooCache("localhost", 120000)).andReturn(zc);
+    expect(zcf.getZooCache(eq("localhost"), eq(120000), isA(Watcher.class))).andReturn(zc);
     replay(zcf);
 
     ZooConfiguration c = zconff.getInstance(instance, zcf, parent);
diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
index 44cf76b5be..23c6318f0f 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java
@@ -177,6 +177,11 @@ public void getProperties(Map<String,String> props, Predicate<String>
filter) {
             }
           }
         }
+
+        @Override
+        public long getUpdateCount() {
+          return 0;
+        }
       };
     }
   }
diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java
b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java
index 1f622f41a8..10880a6014 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancerTest.java
@@ -185,6 +185,11 @@ public void getProperties(Map<String,String> props, Predicate<String>
filter) {
               }
             }
           }
+
+          @Override
+          public long getUpdateCount() {
+            return 0;
+          }
         };
       }
     });
@@ -256,6 +261,11 @@ public void getProperties(Map<String,String> props, Predicate<String>
filter) {
               }
             }
           }
+
+          @Override
+          public long getUpdateCount() {
+            return 0;
+          }
         };
       }
     });
diff --git a/server/base/src/test/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandlerTest.java
b/server/base/src/test/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandlerTest.java
index 52eee250da..39f7e5620c 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandlerTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/rpc/TCredentialsUpdatingInvocationHandlerTest.java
@@ -61,6 +61,11 @@ public String get(Property property) {
       public void getProperties(Map<String,String> props, Predicate<String> filter)
{
         cc.getProperties(props, filter);
       }
+
+      @Override
+      public long getUpdateCount() {
+        return cc.getUpdateCount();
+      }
     };
 
     proxy = new TCredentialsUpdatingInvocationHandler<>(new Object(), conf);
diff --git a/server/master/src/main/java/org/apache/accumulo/master/Master.java b/server/master/src/main/java/org/apache/accumulo/master/Master.java
index f48fbed9ba..63a924df27 100644
--- a/server/master/src/main/java/org/apache/accumulo/master/Master.java
+++ b/server/master/src/main/java/org/apache/accumulo/master/Master.java
@@ -610,8 +610,8 @@ public Master(ServerConfigurationFactory config, VolumeManager fs, String
hostna
     try {
       AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig()
{
         @Override
-        public String getVfsContextClasspathProperty(String key) {
-          return getConfiguration().getArbitrarySystemProperty(Property.VFS_CONTEXT_CLASSPATH_PROPERTY,
key);
+        public Map<String,String> getVfsContextClasspathProperties() {
+          return getConfiguration().getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
         }
       });
     } catch (IOException e) {
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 8cc9d42014..98a538eb97 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -2806,8 +2806,8 @@ public void config(String hostname) {
     try {
       AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig()
{
         @Override
-        public String getVfsContextClasspathProperty(String key) {
-          return getConfiguration().getArbitrarySystemProperty(Property.VFS_CONTEXT_CLASSPATH_PROPERTY,
key);
+        public Map<String,String> getVfsContextClasspathProperties() {
+          return getConfiguration().getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
         }
       });
     } catch (IOException e) {
diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java
index 940d1eb330..03b83eae6c 100644
--- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java
+++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java
@@ -563,10 +563,15 @@ public ClassLoader getClassLoader(final CommandLine cl, final Shell
shellState)
 
         AccumuloVFSClassLoader.getContextManager().setContextConfig(new ContextManager.DefaultContextsConfig()
{
           @Override
-          public String getVfsContextClasspathProperty(String key) {
-            return systemConfig.get(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey() + key);
+          public Map<String,String> getVfsContextClasspathProperties() {
+            Map<String,String> filteredMap = new HashMap<>();
+            for (Entry<String,String> entry : systemConfig.entrySet()) {
+              if (entry.getKey().startsWith(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey()))
{
+                filteredMap.put(entry.getKey(), entry.getValue());
+              }
+            }
+            return filteredMap;
           }
-
         });
       } catch (IllegalStateException ise) {}
 
diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
index e155b3df8e..b39d52a0fa 100644
--- a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
+++ b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java
@@ -99,21 +99,21 @@ public int hashCode() {
 
   public static abstract class DefaultContextsConfig implements ContextsConfig {
 
-    /**
-     * Implementations should prepend {@link AccumuloVFSClassLoader#VFS_CONTEXT_CLASSPATH_PROPERTY}
to the given key.
-     */
-    public abstract String getVfsContextClasspathProperty(String key);
+    public abstract Map<String,String> getVfsContextClasspathProperties();
 
     @Override
     public ContextConfig getContextConfig(String context) {
 
-      String uris = getVfsContextClasspathProperty(context);
+      String prop = AccumuloVFSClassLoader.VFS_CONTEXT_CLASSPATH_PROPERTY + context;
+      Map<String,String> props = getVfsContextClasspathProperties();
+
+      String uris = props.get(prop);
 
       if (uris == null) {
         return null;
       }
 
-      String delegate = getVfsContextClasspathProperty(context + ".delegation");
+      String delegate = props.get(prop + ".delegation");
 
       boolean preDelegate = true;
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message