accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bus...@apache.org
Subject [1/3] accumulo git commit: ACCUMULO-4458 reduce contention on config retrievals.
Date Thu, 15 Sep 2016 17:18:29 GMT
Repository: accumulo
Updated Branches:
  refs/heads/1.8 8c85c32af -> 923c87e2f


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.8
Commit: 48733ce8cb9e89f1243e5e46d792010900086c7b
Parents: 4136bfa
Author: Sean Busbey <busbey@cloudera.com>
Authored: Tue Sep 6 11:52:37 2016 -0500
Committer: Sean Busbey <busbey@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) {


Mime
View raw message