[2/9] accumulo git commit: ACCUMULO-3859 Ensure multiple TableConfiguration instances are not created.

[el...@apache.org]

ACCUMULO-3859 Ensure multiple TableConfiguration instances are not created.

If an instance of a TableConfiguration is cached which isn't the same
instance held by a Tablet, this will result in the Tablet never
receiving updates for constraints and more.


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

Branch: refs/heads/1.7
Commit: f122beb129913a98f3bd32882ef34c8104c35f96
Parents: 6f7bf39
Author: Josh Elser <el...@apache.org>
Authored: Thu May 28 13:28:13 2015 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Thu May 28 21:30:38 2015 -0400

----------------------------------------------------------------------
 .../server/conf/ServerConfigurationFactory.java | 31 +++++++++++++-------
 1 file changed, 20 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/f122beb1/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 96ff5d6..7981f3b 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
@@ -34,14 +34,9 @@ import org.apache.accumulo.fate.zookeeper.ZooCacheFactory;
  */
 public class ServerConfigurationFactory {
 
-  private static final Map<String,Map<String,TableConfiguration>> tableConfigs;
-  private static final Map<String,Map<String,NamespaceConfiguration>> namespaceConfigs;
-  private static final Map<String,Map<String,NamespaceConfiguration>> tableParentConfigs;
-  static {
-    tableConfigs = new HashMap<String,Map<String,TableConfiguration>>(1);
-    namespaceConfigs = new HashMap<String,Map<String,NamespaceConfiguration>>(1);
-    tableParentConfigs = new HashMap<String,Map<String,NamespaceConfiguration>>(1);
-  }
+  private static final Map<String,Map<String,TableConfiguration>> tableConfigs = new HashMap<String,Map<String,TableConfiguration>>(1);
+  private static final Map<String,Map<String,NamespaceConfiguration>> namespaceConfigs = new HashMap<String,Map<String,NamespaceConfiguration>>(1);
+  private static final Map<String,Map<String,NamespaceConfiguration>> tableParentConfigs = new HashMap<String,Map<String,NamespaceConfiguration>>(1);
 
   private static void addInstanceToCaches(String iid) {
     synchronized (tableConfigs) {
@@ -152,13 +147,27 @@ public class ServerConfigurationFactory {
     synchronized (tableConfigs) {
       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.
+
+    // Can't hold the lock during the construction and validation of the config,
+    // which would result in creating multiple objects for the same id.
+    //
+    // ACCUMULO-3859 We _cannot_ all multiple instances to be created for a table. If the TableConfiguration
+    // instance a Tablet holds is not the same as the one cached here, any ConfigurationObservers that
+    // Tablet sets will never see updates from ZooKeeper which means that things like constraints and
+    // default visibility labels will never be updated in a Tablet until it is reloaded.
     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);
+        Map<String,TableConfiguration> configs = tableConfigs.get(instanceID);
+        TableConfiguration existingConf = configs.get(tableId);
+        if (null == existingConf) {
+          // Configuration doesn't exist yet
+          configs.put(tableId, conf);
+        } else {
+          // Someone beat us to the punch, reuse their instance instead of replacing it
+          conf = existingConf;
+        }
       }
     }
     return conf;