Repository: accumulo Updated Branches: refs/heads/1.7 4136bfafc -> 48733ce8c
ACCUMULO-4458 reduce contention on config retrievals. We retrieve configuration values in a few hot code paths, so contention on getting the site configuration and values that are only read from the filesystem once causes a perf impact. We try to reduce that here in a few ways: * Since SiteConfiguration is a singleton, we only retrieve it once when HdfsZooInstance is made instead of on each lookup. * Since HdfsZooInstance is a singleton, we create it once on initialization and avoid synchronizing on retrieval * Since the Hadoop Configuration object uses a single lock for all gets against the shared instance, we copy out the values it reads when the SiteConfiguration singleton is created. This has the side-effect of causing our for-test-only set method to only work on values not set in the Configuration (essentially we treat all configs as final). * Additionally, for those configurations that are read within any of the processing loops of the TabletServer, we have the SiteConfiguration singleton cache the fact that they weren't in the Hadoop Configuration object so that we will default to the parent AccumuloConfiguration without contending for the Hadoop Configuration object lock. Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/48733ce8 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/48733ce8 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/48733ce8 Branch: refs/heads/1.7 Commit: 48733ce8cb9e89f1243e5e46d792010900086c7b Parents: 4136bfa Author: Sean Busbey <bus...@cloudera.com> Authored: Tue Sep 6 11:52:37 2016 -0500 Committer: Sean Busbey <bus...@cloudera.com> Committed: Wed Sep 14 16:35:31 2016 -0700 ---------------------------------------------------------------------- .../org/apache/accumulo/core/conf/Property.java | 7 ++++ .../accumulo/core/conf/SiteConfiguration.java | 24 ++++++++++++- .../accumulo/server/client/HdfsZooInstance.java | 36 +++++++++++++------- 3 files changed, 53 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/48733ce8/core/src/main/java/org/apache/accumulo/core/conf/Property.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index 9d145e1..96adaeb 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -803,6 +803,13 @@ public enum Property { || key.startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()); } + /** + * Properties we check the value of within the TabletServer request handling or maintenance processing loops. + */ + public static final EnumSet<Property> HOT_PATH_PROPERTIES = EnumSet.of(Property.TSERV_CLIENT_TIMEOUT, Property.TSERV_TOTAL_MUTATION_QUEUE_MAX, + Property.TSERV_ARCHIVE_WALOGS, Property.GC_TRASH_IGNORE, Property.TSERV_MAJC_DELAY, Property.TABLE_MINC_LOGS_MAX, Property.TSERV_MAJC_MAXCONCURRENT, + Property.REPLICATION_WORKER_THREADS, Property.TABLE_DURABILITY, Property.INSTANCE_ZK_TIMEOUT, Property.TABLE_CLASSPATH); + private static final EnumSet<Property> fixedProperties = EnumSet.of(Property.TSERV_CLIENTPORT, Property.TSERV_NATIVEMAP_ENABLED, Property.TSERV_SCAN_MAX_OPENFILES, Property.MASTER_CLIENTPORT, Property.GC_PORT); http://git-wip-us.apache.org/repos/asf/accumulo/blob/48733ce8/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java ---------------------------------------------------------------------- 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 b2f5a18..07aaf9f 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 @@ -17,6 +17,8 @@ package org.apache.accumulo.core.conf; import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -45,12 +47,31 @@ public class SiteConfiguration extends AccumuloConfiguration { private static SiteConfiguration instance = null; private static Configuration xmlConfig; + private final Map<String,String> staticConfigs; /** * Not for consumers. Call {@link SiteConfiguration#getInstance(AccumuloConfiguration)} instead */ SiteConfiguration(AccumuloConfiguration parent) { this.parent = parent; + /* + * Make a read-only copy of static configs so we can avoid lock contention on the Hadoop Configuration object + */ + final Configuration conf = getXmlConfig(); + Map<String,String> temp = new HashMap<>((int) (Math.ceil(conf.size() / 0.75f)), 0.75f); + for (Entry<String,String> entry : conf) { + temp.put(entry.getKey(), entry.getValue()); + } + /* + * If any of the configs used in hot codepaths are unset here, set a null so that we'll default to the parent config without contending for the Hadoop + * Configuration object + */ + for (Property hotConfig : Property.HOT_PATH_PROPERTIES) { + if (!(temp.containsKey(hotConfig.getKey()))) { + temp.put(hotConfig.getKey(), null); + } + } + staticConfigs = Collections.unmodifiableMap(temp); } /** @@ -106,7 +127,8 @@ public class SiteConfiguration extends AccumuloConfiguration { } } - String value = getXmlConfig().get(key); + /* Check the available-on-load configs and fall-back to the possibly-update Configuration object. */ + String value = staticConfigs.containsKey(key) ? staticConfigs.get(key) : getXmlConfig().get(key); if (value == null || !property.getType().isValidFormat(value)) { if (value != null) http://git-wip-us.apache.org/repos/asf/accumulo/blob/48733ce8/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java b/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java index 0d7aaf1..2dacf61 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java @@ -23,6 +23,7 @@ import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.client.AccumuloException; @@ -63,20 +64,19 @@ import com.google.common.base.Joiner; */ public class HdfsZooInstance implements Instance { + private final AccumuloConfiguration site = SiteConfiguration.getInstance(); + private HdfsZooInstance() { - AccumuloConfiguration acuConf = SiteConfiguration.getInstance(); - zooCache = new ZooCacheFactory().getZooCache(acuConf.get(Property.INSTANCE_ZK_HOST), (int) acuConf.getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT)); + zooCache = new ZooCacheFactory().getZooCache(site.get(Property.INSTANCE_ZK_HOST), (int) site.getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT)); } - private static HdfsZooInstance cachedHdfsZooInstance = null; + private static final HdfsZooInstance cachedHdfsZooInstance = new HdfsZooInstance(); - public static synchronized Instance getInstance() { - if (cachedHdfsZooInstance == null) - cachedHdfsZooInstance = new HdfsZooInstance(); + public static Instance getInstance() { return cachedHdfsZooInstance; } - private static ZooCache zooCache; + private final ZooCache zooCache; private static String instanceId = null; private static final Logger log = Logger.getLogger(HdfsZooInstance.class); @@ -146,17 +146,17 @@ public class HdfsZooInstance implements Instance { @Override public String getZooKeepers() { - return SiteConfiguration.getInstance().get(Property.INSTANCE_ZK_HOST); + return site.get(Property.INSTANCE_ZK_HOST); } @Override public int getZooKeepersSessionTimeOut() { - return (int) SiteConfiguration.getInstance().getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT); + return (int) site.getTimeInMillis(Property.INSTANCE_ZK_TIMEOUT); } @Override public Connector getConnector(String principal, AuthenticationToken token) throws AccumuloException, AccumuloSecurityException { - return new ConnectorImpl(new ClientContext(this, new Credentials(principal, token), SiteConfiguration.getInstance())); + return new ConnectorImpl(new ClientContext(this, new Credentials(principal, token), site)); } @Deprecated @@ -177,18 +177,28 @@ public class HdfsZooInstance implements Instance { return getConnector(user, TextUtil.getBytes(new Text(pass.toString()))); } - private AccumuloConfiguration conf = null; + private final AtomicReference<AccumuloConfiguration> conf = new AtomicReference<>(); @Deprecated @Override public AccumuloConfiguration getConfiguration() { - return conf = conf == null ? new ServerConfigurationFactory(this).getConfiguration() : conf; + AccumuloConfiguration conf = this.conf.get(); + if (conf == null) { + // conf hasn't been set before, get an instance + conf = new ServerConfigurationFactory(this).getConfiguration(); + // if the shared variable is still null, we're done. + if (!(this.conf.compareAndSet(null, conf))) { + // if it wasn't null, then we need to return the value that won. + conf = this.conf.get(); + } + } + return conf; } @Override @Deprecated public void setConfiguration(AccumuloConfiguration conf) { - this.conf = conf; + this.conf.set(conf); } public static void main(String[] args) {
