You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ec...@apache.org on 2014/12/01 18:58:11 UTC

[1/2] accumulo git commit: ACCUMULO-3372 avoid holding the lock while constructing a cached object

Repository: accumulo
Updated Branches:
  refs/heads/master 211197ce5 -> 191bee180


ACCUMULO-3372 avoid holding the lock while constructing a cached object


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

Branch: refs/heads/master
Commit: 7f20e6aef77f4fe60a93d06ee13e41d8585125cf
Parents: 7ca9c2d
Author: Eric C. Newton <er...@gmail.com>
Authored: Mon Dec 1 12:05:34 2014 -0500
Committer: Eric C. Newton <er...@gmail.com>
Committed: Mon Dec 1 12:05:34 2014 -0500

----------------------------------------------------------------------
 .../server/conf/ServerConfigurationFactory.java | 51 +++++++++++++-------
 1 file changed, 33 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/7f20e6ae/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
index 8e62a87..35b6556 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
@@ -148,15 +148,20 @@ public class ServerConfigurationFactory {
 
   public TableConfiguration getTableConfiguration(String tableId) {
     checkPermissions();
+    TableConfiguration conf;
     synchronized (tableConfigs) {
-      TableConfiguration conf = tableConfigs.get(instanceID).get(tableId);
-      if (conf == null && Tables.exists(instance, tableId)) {
-        conf = new TableConfiguration(instance.getInstanceID(), tableId, getNamespaceConfigurationForTable(tableId));
-        ConfigSanityCheck.validate(conf);
+      conf = tableConfigs.get(instanceID).get(tableId);
+    }
+    // can't hold the lock during the construction and validation of the config, 
+    // which may result in creating multiple objects for the same id, but that's ok.
+    if (conf == null && Tables.exists(instance, tableId)) {
+      conf = new TableConfiguration(instance.getInstanceID(), tableId, getNamespaceConfigurationForTable(tableId));
+      ConfigSanityCheck.validate(conf);
+      synchronized (tableConfigs) {
         tableConfigs.get(instanceID).put(tableId, conf);
       }
-      return conf;
     }
+    return conf;
   }
 
   public TableConfiguration getTableConfiguration(KeyExtent extent) {
@@ -165,31 +170,41 @@ public class ServerConfigurationFactory {
 
   public NamespaceConfiguration getNamespaceConfigurationForTable(String tableId) {
     checkPermissions();
+    NamespaceConfiguration conf;
     synchronized (tableParentConfigs) {
-      NamespaceConfiguration conf = tableParentConfigs.get(instanceID).get(tableId);
-      if (conf == null) {
-        // changed - include instance in constructor call
-        conf = new TableParentConfiguration(tableId, instance, getConfiguration());
-        ConfigSanityCheck.validate(conf);
+      conf = tableParentConfigs.get(instanceID).get(tableId);
+    }
+    // can't hold the lock during the construction and validation of the config, 
+    // which may result in creating multiple objects for the same id, but that's ok.
+    if (conf == null) {
+      // changed - include instance in constructor call
+      conf = new TableParentConfiguration(tableId, instance, getConfiguration());
+      ConfigSanityCheck.validate(conf);
+      synchronized (tableParentConfigs) {
         tableParentConfigs.get(instanceID).put(tableId, conf);
       }
-      return conf;
     }
+    return conf;
   }
 
   public NamespaceConfiguration getNamespaceConfiguration(String namespaceId) {
     checkPermissions();
+    NamespaceConfiguration conf;
+    // can't hold the lock during the construction and validation of the config, 
+    // which may result in creating multiple objects for the same id, but that's ok.
     synchronized (namespaceConfigs) {
-      NamespaceConfiguration conf = namespaceConfigs.get(instanceID).get(namespaceId);
-      if (conf == null) {
-        // changed - include instance in constructor call
-        conf = new NamespaceConfiguration(namespaceId, instance, getConfiguration());
-        conf.setZooCacheFactory(zcf);
-        ConfigSanityCheck.validate(conf);
+      conf = namespaceConfigs.get(instanceID).get(namespaceId);
+    }
+    if (conf == null) {
+      // changed - include instance in constructor call
+      conf = new NamespaceConfiguration(namespaceId, instance, getConfiguration());
+      conf.setZooCacheFactory(zcf);
+      ConfigSanityCheck.validate(conf);
+      synchronized (namespaceConfigs) {
         namespaceConfigs.get(instanceID).put(namespaceId, conf);
       }
-      return conf;
     }
+    return conf;
   }
 
   public Instance getInstance() {


[2/2] accumulo git commit: ACCUMULO-3372 merge 1.6 to master

Posted by ec...@apache.org.
ACCUMULO-3372 merge 1.6 to master


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

Branch: refs/heads/master
Commit: 191bee18078e90f9d7d465c04142e4612404c9f9
Parents: 211197c 7f20e6a
Author: Eric C. Newton <er...@gmail.com>
Authored: Mon Dec 1 12:35:49 2014 -0500
Committer: Eric C. Newton <er...@gmail.com>
Committed: Mon Dec 1 12:35:49 2014 -0500

----------------------------------------------------------------------
 .../server/conf/ServerConfigurationFactory.java | 51 +++++++++++++-------
 1 file changed, 33 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/191bee18/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
----------------------------------------------------------------------
diff --cc server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
index 8bcb5a7,35b6556..128f74e
--- a/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/conf/ServerConfigurationFactory.java
@@@ -147,56 -146,67 +147,71 @@@ public class ServerConfigurationFactor
      return systemConfig;
    }
  
 +  @Override
    public TableConfiguration getTableConfiguration(String tableId) {
      checkPermissions();
+     TableConfiguration conf;
      synchronized (tableConfigs) {
-       TableConfiguration conf = tableConfigs.get(instanceID).get(tableId);
-       if (conf == null && Tables.exists(instance, tableId)) {
+       conf = tableConfigs.get(instanceID).get(tableId);
+     }
+     // can't hold the lock during the construction and validation of the config, 
+     // which may result in creating multiple objects for the same id, but that's ok.
+     if (conf == null && Tables.exists(instance, tableId)) {
 -      conf = new TableConfiguration(instance.getInstanceID(), tableId, getNamespaceConfigurationForTable(tableId));
 -      ConfigSanityCheck.validate(conf);
 -      synchronized (tableConfigs) {
 -        tableConfigs.get(instanceID).put(tableId, conf);
 -      }
 +        conf = new TableConfiguration(instance, tableId, getNamespaceConfigurationForTable(tableId));
 +        ConfigSanityCheck.validate(conf);
-         tableConfigs.get(instanceID).put(tableId, conf);
-       }
-       return conf;
++        synchronized (tableConfigs) {
++          tableConfigs.get(instanceID).put(tableId, conf);
++        }
      }
+     return conf;
    }
  
 +  @Override
    public TableConfiguration getTableConfiguration(KeyExtent extent) {
      return getTableConfiguration(extent.getTableId().toString());
    }
  
    public NamespaceConfiguration getNamespaceConfigurationForTable(String tableId) {
      checkPermissions();
+     NamespaceConfiguration conf;
      synchronized (tableParentConfigs) {
-       NamespaceConfiguration conf = tableParentConfigs.get(instanceID).get(tableId);
-       if (conf == null) {
-         // changed - include instance in constructor call
-         conf = new TableParentConfiguration(tableId, instance, getConfiguration());
-         ConfigSanityCheck.validate(conf);
+       conf = tableParentConfigs.get(instanceID).get(tableId);
+     }
+     // can't hold the lock during the construction and validation of the config, 
+     // which may result in creating multiple objects for the same id, but that's ok.
+     if (conf == null) {
+       // changed - include instance in constructor call
+       conf = new TableParentConfiguration(tableId, instance, getConfiguration());
+       ConfigSanityCheck.validate(conf);
+       synchronized (tableParentConfigs) {
          tableParentConfigs.get(instanceID).put(tableId, conf);
        }
-       return conf;
      }
+     return conf;
    }
  
 +  @Override
    public NamespaceConfiguration getNamespaceConfiguration(String namespaceId) {
      checkPermissions();
+     NamespaceConfiguration conf;
+     // can't hold the lock during the construction and validation of the config, 
+     // which may result in creating multiple objects for the same id, but that's ok.
      synchronized (namespaceConfigs) {
-       NamespaceConfiguration conf = namespaceConfigs.get(instanceID).get(namespaceId);
-       if (conf == null) {
-         // changed - include instance in constructor call
-         conf = new NamespaceConfiguration(namespaceId, instance, getConfiguration());
-         conf.setZooCacheFactory(zcf);
-         ConfigSanityCheck.validate(conf);
+       conf = namespaceConfigs.get(instanceID).get(namespaceId);
+     }
+     if (conf == null) {
+       // changed - include instance in constructor call
+       conf = new NamespaceConfiguration(namespaceId, instance, getConfiguration());
+       conf.setZooCacheFactory(zcf);
+       ConfigSanityCheck.validate(conf);
+       synchronized (namespaceConfigs) {
          namespaceConfigs.get(instanceID).put(namespaceId, conf);
        }
-       return conf;
      }
+     return conf;
    }
  
 +  @Override
    public Instance getInstance() {
      return instance;
    }