accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ktur...@apache.org
Subject [accumulo] branch 1.8 updated: ACCUMULO-4779 Improved performance of get by prefix in config (#372)
Date Mon, 05 Feb 2018 22:07:10 GMT
This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch 1.8
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/1.8 by this push:
     new 3ca3d65  ACCUMULO-4779 Improved performance of get by prefix in config (#372)
3ca3d65 is described below

commit 3ca3d65962e5229a44d67ef46233467d53a7016c
Author: Keith Turner <keith@deenlo.com>
AuthorDate: Mon Feb 5 17:07:07 2018 -0500

    ACCUMULO-4779 Improved performance of get by prefix in config (#372)
    
    This change improves performance by caching config with a prefix.
    
    Also removed custom method to get VFS config.  Now that getting
    config by prefixes is fast, it can used for VFS.
---
 .../accumulo/core/client/impl/ClientContext.java   |   5 -
 .../core/client/mock/MockConfiguration.java        |   5 -
 .../accumulo/core/conf/AccumuloConfiguration.java  |  89 ++++++++----
 .../accumulo/core/conf/ConfigurationCopy.java      |  22 ++-
 .../accumulo/core/conf/DefaultConfiguration.java   |   5 -
 .../accumulo/core/conf/SiteConfiguration.java      |   9 --
 .../core/security/crypto/CryptoModuleFactory.java  |   2 +-
 .../core/conf/AccumuloConfigurationTest.java       | 152 +++++++++++++++++++++
 .../apache/accumulo/fate/zookeeper/ZooCache.java   |  33 +++--
 .../server/conf/NamespaceConfiguration.java        |   5 +
 .../accumulo/server/conf/TableConfiguration.java   |   5 +
 .../accumulo/server/conf/ZooConfiguration.java     |  14 +-
 .../server/conf/ZooConfigurationFactory.java       |  13 +-
 .../server/conf/ZooConfigurationFactoryTest.java   |   5 +-
 .../BaseHostRegexTableLoadBalancerTest.java        |   5 +
 .../balancer/HostRegexTableLoadBalancerTest.java   |  10 ++
 .../TCredentialsUpdatingInvocationHandlerTest.java |   5 +
 .../java/org/apache/accumulo/master/Master.java    |   4 +-
 .../org/apache/accumulo/tserver/TabletServer.java  |   4 +-
 .../main/java/org/apache/accumulo/shell/Shell.java |  11 +-
 .../start/classloader/vfs/ContextManager.java      |  12 +-
 21 files changed, 318 insertions(+), 97 deletions(-)

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 b388f77..9f68d16 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
@@ -176,11 +176,6 @@ public class ClientContext {
     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 a4f6a15..244d6f8 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
@@ -40,11 +40,6 @@ class MockConfiguration extends AccumuloConfiguration {
   }
 
   @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 7e0db72..3bf54be 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.Map.Entry;
 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.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
 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 abstract class AccumuloConfiguration implements Iterable<Entry<String,Str
 
   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.
    *
@@ -194,6 +184,13 @@ public abstract class AccumuloConfiguration implements Iterable<Entry<String,Str
   }
 
   /**
+   * Each time configurations is changes, this counter should increase.
+   */
+  public long getUpdateCount() {
+    return 0;
+  }
+
+  /**
    * Gets all properties under the given prefix in this configuration.
    *
    * @param property
@@ -205,9 +202,41 @@ public abstract class AccumuloConfiguration implements Iterable<Entry<String,Str
   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 df43557..a84765a 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 @@ import com.google.common.base.Predicate;
  * 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>());
 
   /**
@@ -64,11 +65,6 @@ public class ConfigurationCopy extends AccumuloConfiguration {
   }
 
   @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()) {
       if (filter.apply(entry.getKey())) {
@@ -86,7 +82,10 @@ public class ConfigurationCopy extends AccumuloConfiguration {
    *          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 class ConfigurationCopy extends AccumuloConfiguration {
    *          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 994d960..e1ff7e1 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 class DefaultConfiguration extends AccumuloConfiguration {
       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 32b2564..9f047e2 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
@@ -129,15 +129,6 @@ public class SiteConfiguration extends AccumuloConfiguration {
   }
 
   @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 79db306..f724b7b 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 class CryptoModuleFactory {
 
   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 f12ba63..ba373ab 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 class AccumuloConfigurationTest {
     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 801ee2c..3daf20b 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 @@ public class ZooCache {
     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 class ZooCache {
             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 @@ public class ZooCache {
       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 @@ public class ZooCache {
       childrenCache.remove(zPath);
       statCache.remove(zPath);
 
-      immutableCache = new ImmutableCacheCopies(cache, statCache, childrenCache);
+      immutableCache = new ImmutableCacheCopies(++updateCount, cache, statCache, childrenCache);
     } finally {
       cacheWriteLock.unlock();
     }
@@ -440,13 +446,20 @@ public class ZooCache {
       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.
    *
    * @param zPath
@@ -508,7 +521,7 @@ public class ZooCache {
           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 1ca083e..51783f6 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 class NamespaceConfiguration extends ObservableConfiguration {
     // 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 6c53c5b..8e68d5f 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 class TableConfiguration extends ObservableConfiguration {
   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 cb7bc98..79c3806 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
@@ -79,15 +79,6 @@ public class ZooConfiguration extends AccumuloConfiguration {
   }
 
   @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)) {
       if (fixedProps.containsKey(property.getKey())) {
@@ -129,4 +120,9 @@ public class ZooConfiguration extends AccumuloConfiguration {
       }
     }
   }
+
+  @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 0cfbff8..ce62b93 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.Accumulo;
 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 @@ class ZooConfigurationFactory {
       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 b069163..4c0e842 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.client.Instance;
 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 class ZooConfigurationFactoryTest {
     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 44cf76b..23c6318 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 abstract class BaseHostRegexTableLoadBalancerTest extends HostRegexTableL
             }
           }
         }
+
+        @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 1f622f4..10880a6 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 class HostRegexTableLoadBalancerTest extends BaseHostRegexTableLoadBalanc
               }
             }
           }
+
+          @Override
+          public long getUpdateCount() {
+            return 0;
+          }
         };
       }
     });
@@ -256,6 +261,11 @@ public class HostRegexTableLoadBalancerTest extends BaseHostRegexTableLoadBalanc
               }
             }
           }
+
+          @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 52eee25..39f7e56 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 class TCredentialsUpdatingInvocationHandlerTest {
       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 f48fbed..63a924d 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 class Master extends AccumuloServerContext implements LiveTServerSet.List
     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 8cc9d42..98a538e 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 class TabletServer extends AccumuloServerContext implements Runnable
{
     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 940d1eb..03b83ea 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 class Shell extends ShellOptions implements KeywordExecutable
{
 
         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 e155b3d..b39d52a 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 class ContextManager {
 
   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;
 

-- 
To stop receiving notification emails like this one, please contact
kturner@apache.org.

Mime
View raw message