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/44cd4703 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/44cd4703 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/44cd4703 Branch: refs/heads/1.7 Commit: 44cd47035a5d0a306717a0e018f771fdb68ed65c Parents: 1ad4bfa Author: Josh Elser <els...@apache.org> Authored: Thu May 28 13:28:13 2015 -0400 Committer: Josh Elser <els...@apache.org> Committed: Thu May 28 21:14:20 2015 -0400 ---------------------------------------------------------------------- .../server/conf/ServerConfigurationFactory.java | 31 +++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/44cd4703/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 a45ff56..4ad1d5f 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 extends ServerConfiguration { - 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) { @@ -154,13 +149,27 @@ public class ServerConfigurationFactory extends ServerConfiguration { 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, 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;
