You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by kt...@apache.org on 2018/02/05 22:07:10 UTC
[accumulo] branch 1.8 updated: ACCUMULO-4779 Improved performance
of get by prefix in config (#372)
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 <ke...@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.