You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by bu...@apache.org on 2016/09/15 17:18:29 UTC

[1/3] accumulo git commit: ACCUMULO-4458 reduce contention on config retrievals.

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 <bu...@cloudera.com>
Authored: Tue Sep 6 11:52:37 2016 -0500
Committer: Sean Busbey <bu...@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) {


[2/3] accumulo git commit: Merge branch '1.7' into 1.8

Posted by bu...@apache.org.
Merge branch '1.7' into 1.8

 Conflicts:
	server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/1bf8a636
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/1bf8a636
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/1bf8a636

Branch: refs/heads/1.8
Commit: 1bf8a6362c685f316d04facb26faa29939cad9ef
Parents: 8c85c32 48733ce
Author: Sean Busbey <bu...@cloudera.com>
Authored: Thu Sep 15 08:41:13 2016 -0700
Committer: Sean Busbey <bu...@cloudera.com>
Committed: Thu Sep 15 08:41:13 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/1bf8a636/core/src/main/java/org/apache/accumulo/core/conf/Property.java
----------------------------------------------------------------------
diff --cc core/src/main/java/org/apache/accumulo/core/conf/Property.java
index cdaebef,96adaeb..34ef953
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@@ -822,9 -800,16 +822,16 @@@ public enum Property 
      return validTableProperties.contains(key) || key.startsWith(Property.TABLE_CONSTRAINT_PREFIX.getKey())
          || key.startsWith(Property.TABLE_ITERATOR_PREFIX.getKey()) || key.startsWith(Property.TABLE_LOCALITY_GROUP_PREFIX.getKey())
          || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey()) || key.startsWith(Property.TABLE_REPLICATION_TARGET.getKey())
 -        || key.startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey());
 +        || key.startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()) || key.startsWith(TABLE_SAMPLER_OPTS.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/1bf8a636/server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java
----------------------------------------------------------------------
diff --cc server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java
index e4e73d2,2dacf61..49bf1f4
--- 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,7 -23,7 +23,8 @@@ import java.nio.ByteBuffer
  import java.util.Collections;
  import java.util.List;
  import java.util.UUID;
 +import java.util.concurrent.TimeUnit;
+ import java.util.concurrent.atomic.AtomicReference;
  
  import org.apache.accumulo.core.Constants;
  import org.apache.accumulo.core.client.AccumuloException;
@@@ -76,9 -76,9 +76,9 @@@ public class HdfsZooInstance implement
      return cachedHdfsZooInstance;
    }
  
-   private static ZooCache zooCache;
+   private final ZooCache zooCache;
    private static String instanceId = null;
 -  private static final Logger log = Logger.getLogger(HdfsZooInstance.class);
 +  private static final Logger log = LoggerFactory.getLogger(HdfsZooInstance.class);
  
    @Override
    public String getRootTabletLocation() {


[3/3] accumulo git commit: ACCUMULO-4458 addendum for 1.8

Posted by bu...@apache.org.
ACCUMULO-4458 addendum for 1.8

- Adds an additional property to HOT_PATH_PROPERTIES based on use in 1.8 tablet server.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/923c87e2
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/923c87e2
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/923c87e2

Branch: refs/heads/1.8
Commit: 923c87e2fbdfd49409e8099293cc87371da8f6fd
Parents: 1bf8a63
Author: Sean Busbey <bu...@cloudera.com>
Authored: Thu Sep 15 08:47:24 2016 -0700
Committer: Sean Busbey <bu...@cloudera.com>
Committed: Thu Sep 15 08:47:24 2016 -0700

----------------------------------------------------------------------
 core/src/main/java/org/apache/accumulo/core/conf/Property.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/923c87e2/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 34ef953..ede1c6f 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
@@ -830,7 +830,8 @@ public enum Property {
    */
   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);
+      Property.REPLICATION_WORKER_THREADS, Property.TABLE_DURABILITY, Property.INSTANCE_ZK_TIMEOUT, Property.TABLE_CLASSPATH,
+      Property.MASTER_METADATA_SUSPENDABLE);
 
   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);