AMBARI-18906 - Remove Unnecessary Locks Inside Of Config Business Object Implementations (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/a6639a7c Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/a6639a7c Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/a6639a7c Branch: refs/heads/branch-feature-AMBARI-18456 Commit: a6639a7c72043ff7bda03e6ba305913c7503193a Parents: 5d7824e Author: Jonathan Hurley <jhur...@hortonworks.com> Authored: Wed Nov 16 08:35:20 2016 -0500 Committer: Jonathan Hurley <jhur...@hortonworks.com> Committed: Thu Nov 17 11:20:55 2016 -0500 ---------------------------------------------------------------------- .../AmbariManagementControllerImpl.java | 13 +- .../internal/ConfigGroupResourceProvider.java | 13 +- .../serveraction/upgrades/ConfigureAction.java | 16 +- .../serveraction/upgrades/FixLzoCodecPath.java | 16 +- .../upgrades/FixOozieAdminUsers.java | 9 +- .../upgrades/HBaseConfigCalculation.java | 14 +- .../HBaseEnvMaxDirectMemorySizeAction.java | 13 +- .../upgrades/HiveEnvClasspathAction.java | 13 +- .../upgrades/HiveZKQuorumConfigAction.java | 2 +- .../upgrades/OozieConfigCalculation.java | 13 +- .../upgrades/RangerConfigCalculation.java | 4 +- .../RangerKerberosConfigCalculation.java | 20 +- .../upgrades/SparkShufflePropertyConfig.java | 3 +- .../upgrades/YarnConfigCalculation.java | 2 +- .../org/apache/ambari/server/state/Config.java | 22 +- .../ambari/server/state/ConfigFactory.java | 20 +- .../apache/ambari/server/state/ConfigImpl.java | 474 +++++++++---------- .../state/configgroup/ConfigGroupImpl.java | 38 +- .../ambari/server/topology/AmbariContext.java | 23 +- .../ambari/server/update/HostUpdateHelper.java | 10 +- .../ExecutionCommandWrapperTest.java | 17 +- .../TestActionSchedulerThreading.java | 19 +- .../server/agent/HeartbeatTestHelper.java | 6 +- .../server/agent/TestHeartbeatMonitor.java | 13 +- .../configuration/RecoveryConfigHelperTest.java | 2 +- .../AmbariManagementControllerImplTest.java | 22 +- .../AmbariManagementControllerTest.java | 107 ++--- .../UpgradeResourceProviderHDP22Test.java | 14 +- .../internal/UpgradeResourceProviderTest.java | 15 +- .../ComponentVersionCheckActionTest.java | 19 +- .../upgrades/ConfigureActionTest.java | 96 +--- .../upgrades/FixOozieAdminUsersTest.java | 76 ++- .../HBaseEnvMaxDirectMemorySizeActionTest.java | 187 ++++---- .../upgrades/HiveEnvClasspathActionTest.java | 148 +++--- .../upgrades/HiveZKQuorumConfigActionTest.java | 2 +- .../upgrades/KerberosKeytabsActionTest.java | 28 +- .../upgrades/RangerConfigCalculationTest.java | 72 +-- .../RangerKerberosConfigCalculationTest.java | 173 ++----- .../SparkShufflePropertyConfigTest.java | 30 +- .../upgrades/UpgradeActionTest.java | 28 +- .../ambari/server/state/ConfigGroupTest.java | 8 +- .../ambari/server/state/ConfigHelperTest.java | 48 +- .../state/alerts/AlertReceivedListenerTest.java | 8 +- .../state/cluster/ClusterDeadlockTest.java | 17 +- .../server/state/cluster/ClusterTest.java | 126 ++--- .../server/state/cluster/ClustersTest.java | 8 +- ...omponentHostConcurrentWriteDeadlockTest.java | 9 +- .../ambari/server/state/host/HostTest.java | 6 +- .../svccomphost/ServiceComponentHostTest.java | 21 +- .../server/topology/AmbariContextTest.java | 37 +- .../server/update/HostUpdateHelperTest.java | 40 +- .../ambari/server/utils/StageUtilsTest.java | 4 + 52 files changed, 846 insertions(+), 1298 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java index b04fdd7..7da1034 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java @@ -54,7 +54,6 @@ import java.util.EnumMap; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -79,10 +78,10 @@ import org.apache.ambari.server.ServiceComponentNotFoundException; import org.apache.ambari.server.ServiceNotFoundException; import org.apache.ambari.server.StackAccessException; import org.apache.ambari.server.actionmanager.ActionManager; +import org.apache.ambari.server.actionmanager.CommandExecutionType; import org.apache.ambari.server.actionmanager.HostRoleCommand; import org.apache.ambari.server.actionmanager.RequestFactory; import org.apache.ambari.server.actionmanager.Stage; -import org.apache.ambari.server.actionmanager.CommandExecutionType; import org.apache.ambari.server.actionmanager.StageFactory; import org.apache.ambari.server.agent.ExecutionCommand; import org.apache.ambari.server.agent.ExecutionCommand.KeyNames; @@ -935,17 +934,11 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle @Override public Config createConfig(Cluster cluster, String type, Map<String, String> properties, String versionTag, Map<String, Map<String, String>> propertiesAttributes) { - Config config = configFactory.createNew(cluster, type, - properties, propertiesAttributes); - if (!StringUtils.isEmpty(versionTag)) { - config.setTag(versionTag); - } - - config.persist(); + Config config = configFactory.createNew(cluster, type, versionTag, properties, + propertiesAttributes); cluster.addConfig(config); - return config; } http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java index 96bb8f9..b957f0a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java @@ -48,6 +48,7 @@ import org.apache.ambari.server.security.authorization.RoleAuthorization; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; +import org.apache.ambari.server.state.ConfigFactory; import org.apache.ambari.server.state.ConfigImpl; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.configgroup.ConfigGroup; @@ -100,6 +101,12 @@ public class ConfigGroupResourceProvider extends @Inject private static HostDAO hostDAO; + + /** + * Used for creating {@link Config} instances to return in the REST response. + */ + @Inject + private static ConfigFactory configFactory; /** * Create a new resource provider for the given management controller. @@ -781,11 +788,7 @@ public class ConfigGroupResourceProvider extends } } - Config config = new ConfigImpl(type); - config.setTag(tag); - config.setProperties(configProperties); - config.setPropertiesAttributes(configAttributes); - + Config config = configFactory.createReadOnly(type, tag, configProperties, configAttributes); configurations.put(config.getType(), config); } } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java index 5459ddb..97280ee 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java @@ -451,7 +451,7 @@ public class ConfigureAction extends AbstractServerAction { // of creating a whole new history record since it was already done if (!targetStack.equals(currentStack) && targetStack.equals(configStack)) { config.setProperties(newValues); - config.persist(false); + config.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", outputBuffer.toString(), ""); } @@ -570,8 +570,9 @@ public class ConfigureAction extends AbstractServerAction { for(Replace replacement: replacements){ if(isOperationAllowed(cluster, configType, replacement.key, - replacement.ifKey, replacement.ifType, replacement.ifValue, replacement.ifKeyState)) + replacement.ifKey, replacement.ifType, replacement.ifValue, replacement.ifKeyState)) { allowedReplacements.add(replacement); + } } return allowedReplacements; @@ -582,8 +583,9 @@ public class ConfigureAction extends AbstractServerAction { for(ConfigurationKeyValue configurationKeyValue: sets){ if(isOperationAllowed(cluster, configType, configurationKeyValue.key, - configurationKeyValue.ifKey, configurationKeyValue.ifType, configurationKeyValue.ifValue, configurationKeyValue.ifKeyState)) + configurationKeyValue.ifKey, configurationKeyValue.ifType, configurationKeyValue.ifValue, configurationKeyValue.ifKeyState)) { allowedSets.add(configurationKeyValue); + } } return allowedSets; @@ -593,14 +595,16 @@ public class ConfigureAction extends AbstractServerAction { List<Transfer> allowedTransfers = new ArrayList<>(); for (Transfer transfer : transfers) { String key = ""; - if(transfer.operation == TransferOperation.DELETE) + if(transfer.operation == TransferOperation.DELETE) { key = transfer.deleteKey; - else + } else { key = transfer.fromKey; + } if(isOperationAllowed(cluster, configType, key, - transfer.ifKey, transfer.ifType, transfer.ifValue, transfer.ifKeyState)) + transfer.ifKey, transfer.ifType, transfer.ifValue, transfer.ifKeyState)) { allowedTransfers.add(transfer); + } } return allowedTransfers; http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixLzoCodecPath.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixLzoCodecPath.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixLzoCodecPath.java index ffa21ab..4833729 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixLzoCodecPath.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixLzoCodecPath.java @@ -18,7 +18,11 @@ package org.apache.ambari.server.serveraction.upgrades; -import com.google.inject.Inject; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentMap; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; @@ -28,13 +32,7 @@ import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; import org.apache.commons.lang.StringUtils; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import com.google.inject.Inject; /** * During stack upgrade, update lzo codec path in mapreduce.application.classpath and @@ -78,7 +76,7 @@ public class FixLzoCodecPath extends AbstractServerAction { } } config.setProperties(properties); - config.persist(false); + config.save(); } if (modifiedProperties.isEmpty()) { return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsers.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsers.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsers.java index 3a06476..75588d5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsers.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/FixOozieAdminUsers.java @@ -18,7 +18,9 @@ package org.apache.ambari.server.serveraction.upgrades; -import com.google.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentMap; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; @@ -28,8 +30,7 @@ import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; import org.apache.commons.lang.StringUtils; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; +import com.google.inject.Inject; /** * During stack upgrade, update lzo codec path in mapreduce.application.classpath and @@ -86,7 +87,7 @@ public class FixOozieAdminUsers extends AbstractServerAction { oozieProperties.put(OOZIE_ADMIN_USERS_PROP, newOozieAdminUsers); oozieConfig.setProperties(oozieProperties); - oozieConfig.persist(false); + oozieConfig.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("Set oozie admin users to %s", newOozieAdminUsers), ""); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseConfigCalculation.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseConfigCalculation.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseConfigCalculation.java index 7f6d4b1..739dd7e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseConfigCalculation.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseConfigCalculation.java @@ -18,7 +18,10 @@ package org.apache.ambari.server.serveraction.upgrades; -import com.google.inject.Inject; +import java.math.BigDecimal; +import java.util.Map; +import java.util.concurrent.ConcurrentMap; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; @@ -27,9 +30,7 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; -import java.math.BigDecimal; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; +import com.google.inject.Inject; /** * Computes HBase properties. This class is only used when moving from @@ -79,8 +80,9 @@ public class HBaseConfigCalculation extends AbstractServerAction { "Upper or lower memstore limit setting value is malformed, skipping", ""); } - if (lowerLimit.scale() < 2) //make sure result will have at least 2 digits after decimal point + if (lowerLimit.scale() < 2) { lowerLimit = lowerLimit.setScale(2, BigDecimal.ROUND_HALF_UP); + } BigDecimal lowerLimitNew = lowerLimit.divide(upperLimit, BigDecimal.ROUND_HALF_UP); properties.put(NEW_LOWER_LIMIT_PROPERTY_NAME, lowerLimitNew.toString()); @@ -90,7 +92,7 @@ public class HBaseConfigCalculation extends AbstractServerAction { properties.remove(OLD_LOWER_LIMIT_PROPERTY_NAME); config.setProperties(properties); - config.persist(false); + config.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("%s was set to %s", NEW_LOWER_LIMIT_PROPERTY_NAME, lowerLimitNew.toString()), ""); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeAction.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeAction.java index b238bca..fb15555 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeAction.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HBaseEnvMaxDirectMemorySizeAction.java @@ -18,7 +18,11 @@ package org.apache.ambari.server.serveraction.upgrades; -import com.google.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; @@ -27,10 +31,7 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import com.google.inject.Inject; /** * Computes HBase Env content property. @@ -79,7 +80,7 @@ public class HBaseEnvMaxDirectMemorySizeAction extends AbstractServerAction { properties.put(CONTENT_NAME, appendedContent); config.setProperties(properties); - config.persist(false); + config.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("The %s/%s property was appended with %s", SOURCE_CONFIG_TYPE, CONTENT_NAME, APPEND_CONTENT_LINE),""); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathAction.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathAction.java index 0e10160..c5000bf 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathAction.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveEnvClasspathAction.java @@ -18,7 +18,11 @@ package org.apache.ambari.server.serveraction.upgrades; -import com.google.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; @@ -27,10 +31,7 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import com.google.inject.Inject; /** * Append hive-env config type with HIVE_HOME and HIVE_CONF_DIR variables if they are absent @@ -103,7 +104,7 @@ public class HiveEnvClasspathAction extends AbstractServerAction { } config.setProperties(properties); - config.persist(false); + config.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("Added %s, %s to content at %s", HIVE_CONF_DIR, HIVE_HOME, TARGET_CONFIG_TYPE), ""); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java index 0ade30b..7ebad08 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/HiveZKQuorumConfigAction.java @@ -85,7 +85,7 @@ public class HiveZKQuorumConfigAction extends AbstractServerAction { hiveSiteProperties.put(HIVE_SITE_ZK_CONNECT_STRING, zookeeperQuorum); hiveSite.setProperties(hiveSiteProperties); - hiveSite.persist(false); + hiveSite.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("Successfully set %s and %s in %s", HIVE_SITE_ZK_QUORUM, http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/OozieConfigCalculation.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/OozieConfigCalculation.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/OozieConfigCalculation.java index 4da67ca..9b8a7dc 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/OozieConfigCalculation.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/OozieConfigCalculation.java @@ -18,7 +18,11 @@ package org.apache.ambari.server.serveraction.upgrades; -import com.google.inject.Inject; +import java.util.Map; +import java.util.concurrent.ConcurrentMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; @@ -27,10 +31,7 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; -import java.util.Map; -import java.util.concurrent.ConcurrentMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import com.google.inject.Inject; /** * Changes oozie-env during upgrade (adds -Dhdp.version to $HADOOP_OPTS variable) @@ -67,7 +68,7 @@ public class OozieConfigCalculation extends AbstractServerAction { } config.setProperties(properties); - config.persist(false); + config.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("Added -Dhdp.version to $HADOOP_OPTS variable at %s", TARGET_CONFIG_TYPE), ""); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculation.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculation.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculation.java index ff4a20e..8e0161b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculation.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerConfigCalculation.java @@ -141,13 +141,13 @@ public class RangerConfigCalculation extends AbstractServerAction { targetValues.put("ranger.jpa.audit.jdbc.dialect", dialect); config.setProperties(targetValues); - config.persist(false); + config.save(); config = cluster.getDesiredConfigByType(RANGER_ENV_CONFIG_TYPE); targetValues = config.getProperties(); targetValues.put("ranger_privelege_user_jdbc_url", userJDBCUrl); config.setProperties(targetValues); - config.persist(false); + config.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", stdout.toString(), ""); } http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculation.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculation.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculation.java index ba0da79..c059c9e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculation.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/RangerKerberosConfigCalculation.java @@ -87,7 +87,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != hadoopUser) { targetValues.put(RANGER_PLUGINS_HDFS_SERVICE_USER, hadoopUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_HDFS_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "hdfs_user", HADOOP_ENV_CONFIG_TYPE); @@ -104,7 +104,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != hiveUser) { targetValues.put(RANGER_PLUGINS_HIVE_SERVICE_USER, hiveUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_HIVE_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "hive_user", HIVE_ENV_CONFIG_TYPE); @@ -121,7 +121,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != yarnUser) { targetValues.put(RANGER_PLUGINS_YARN_SERVICE_USER, yarnUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_YARN_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "yarn_user", YARN_ENV_CONFIG_TYPE); @@ -138,7 +138,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != hbaseUser) { targetValues.put(RANGER_PLUGINS_HBASE_SERVICE_USER, hbaseUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_HBASE_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "hbase_user", HBASE_ENV_CONFIG_TYPE); @@ -155,7 +155,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != knoxUser) { targetValues.put(RANGER_PLUGINS_KNOX_SERVICE_USER, knoxUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_KNOX_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "knox_user", KNOX_ENV_CONFIG_TYPE); @@ -190,7 +190,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { } targetValues.put(RANGER_PLUGINS_STORM_SERVICE_USER, stormValue); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_STORM_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "storm_user", STORM_ENV_CONFIG_TYPE); @@ -207,7 +207,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != kafkaUser) { targetValues.put(RANGER_PLUGINS_KAFKA_SERVICE_USER, kafkaUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_KAFKA_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "kafka_user", KAFKA_ENV_CONFIG_TYPE); @@ -224,7 +224,7 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != rangerKmsUser) { targetValues.put(RANGER_PLUGINS_KMS_SERVICE_USER, rangerKmsUser); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_PLUGINS_KMS_SERVICE_USER); } else { errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "kms_user", RANGER_KMS_ENV_CONFIG_TYPE); @@ -243,10 +243,10 @@ public class RangerKerberosConfigCalculation extends AbstractServerAction { if (null != spnegoKeytab) { targetValues.put(RANGER_SPNEGO_KEYTAB, spnegoKeytab); rangerAdminconfig.setProperties(targetValues); - rangerAdminconfig.persist(false); + rangerAdminconfig.save(); sucessMsg = sucessMsg + MessageFormat.format("{0}\n", RANGER_SPNEGO_KEYTAB); } else { - errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "dfs.web.authentication.kerberos.keytab", HDFS_SITE_CONFIG_TYPE); + errMsg = errMsg + MessageFormat.format("{0} not found in {1}\n", "dfs.web.authentication.kerberos.keytab", HDFS_SITE_CONFIG_TYPE); } } else { http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfig.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfig.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfig.java index 299a373..b1aa6e1 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfig.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/SparkShufflePropertyConfig.java @@ -25,7 +25,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentMap; import org.apache.ambari.server.AmbariException; -import org.apache.ambari.server.ServiceNotFoundException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.agent.CommandReport; import org.apache.ambari.server.serveraction.AbstractServerAction; @@ -89,7 +88,7 @@ public class SparkShufflePropertyConfig extends AbstractServerAction { yarnSiteProperties.put(YARN_NODEMANAGER_AUX_SERVICES, newAuxServices); yarnSiteProperties.put(YARN_NODEMANAGER_AUX_SERVICES_SPARK_SHUFFLE_CLASS, YARN_NODEMANAGER_AUX_SERVICES_SPARK_SHUFFLE_CLASS_VALUE); yarnSiteConfig.setProperties(yarnSiteProperties); - yarnSiteConfig.persist(false); + yarnSiteConfig.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("%s was set from %s to %s. %s was set to %s", http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/YarnConfigCalculation.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/YarnConfigCalculation.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/YarnConfigCalculation.java index feefcaf..d638858 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/YarnConfigCalculation.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/YarnConfigCalculation.java @@ -67,7 +67,7 @@ public class YarnConfigCalculation extends AbstractServerAction { yarnSiteProperties.put(YARN_RM_ZK_ADDRESS_PROPERTY_NAME, zkServersStr); yarnSiteProperties.put(HADOOP_REGISTRY_ZK_QUORUM_PROPERTY_NAME, zkServersStr); yarnSiteConfig.setProperties(yarnSiteProperties); - yarnSiteConfig.persist(false); + yarnSiteConfig.save(); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", String.format("%s was set from %s to %s. %s was set from %s to %s", http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java index b35aad9..67570f4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Config.java @@ -30,8 +30,6 @@ public interface Config { void setPropertiesTypes(Map<PropertyInfo.PropertyType, Set<String>> propertiesTypes); - void setStackId(StackId stackId); - /** * @return Config Type */ @@ -66,18 +64,6 @@ public interface Config { public Map<String, Map<String, String>> getPropertiesAttributes(); /** - * Change the version tag - * @param versionTag - */ - public void setTag(String versionTag); - - /** - * Set config version - * @param version - */ - public void setVersion(Long version); - - /** * Replace properties with new provided set * @param properties Property Map to replace existing one */ @@ -110,11 +96,5 @@ public interface Config { /** * Persist the configuration. */ - public void persist(); - - /** - * Persist the configuration, optionally creating a new config entity. - */ - public void persist(boolean newConfig); - + public void save(); } http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigFactory.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigFactory.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigFactory.java index eaf68aa..d6cd997 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigFactory.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigFactory.java @@ -27,18 +27,20 @@ import com.google.inject.assistedinject.Assisted; * Factory for creating configuration objects using {@link Assisted} constructor parameters */ public interface ConfigFactory { - + /** * Creates a new {@link Config} object using provided values. * * @param cluster * @param type + * @param tag * @param map * @param mapAttributes * @return */ - Config createNew(Cluster cluster, String type, Map<String, String> map, Map<String, Map<String, String>> mapAttributes); - + Config createNew(Cluster cluster, @Assisted("type") String type, @Assisted("tag") String tag, + Map<String, String> map, Map<String, Map<String, String>> mapAttributes); + /** * Creates a new {@link Config} object using provided entity * @@ -48,4 +50,16 @@ public interface ConfigFactory { */ Config createExisting(Cluster cluster, ClusterConfigEntity entity); + /** + * Creates a read-only instance of a {@link Config} suitable for returning in + * REST responses. + * + * @param type + * @param tag + * @param map + * @param mapAttributes + * @return + */ + Config createReadOnly(@Assisted("type") String type, @Assisted("tag") String tag, + Map<String, String> map, Map<String, Map<String, String>> mapAttributes); } http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java index 28bcd5f..e68839f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigImpl.java @@ -18,28 +18,28 @@ package org.apache.ambari.server.state; -import java.util.Collections; -import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.ReentrantReadWriteLock; +import javax.annotation.Nullable; + import org.apache.ambari.server.events.ClusterConfigChangedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.orm.dao.ClusterDAO; import org.apache.ambari.server.orm.dao.ServiceConfigDAO; import org.apache.ambari.server.orm.entities.ClusterConfigEntity; import org.apache.ambari.server.orm.entities.ClusterEntity; +import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; import com.google.inject.Inject; -import com.google.inject.Injector; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; import com.google.inject.persist.Transactional; @@ -52,50 +52,101 @@ public class ConfigImpl implements Config { public static final String GENERATED_TAG_PREFIX = "generatedTag_"; - private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); + private final long configId; + private final Cluster cluster; + private final StackId stackId; + private final String type; + private final String tag; + private final Long version; - private Cluster cluster; - private StackId stackId; - private String type; - private volatile String tag; - private volatile Long version; - private volatile Map<String, String> properties; - private volatile Map<String, Map<String, String>> propertiesAttributes; - private ClusterConfigEntity entity; - private volatile Map<PropertyInfo.PropertyType, Set<String>> propertiesTypes; + /** + * The properties of this configuration. This cannot be a + * {@link ConcurrentMap} since we allow null values. Therefore, it must be + * synchronized externally. + */ + private Map<String, String> properties; - @Inject - private ClusterDAO clusterDAO; + /** + * A lock for reading/writing of {@link #properties} concurrently. + * + * @see #properties + */ + private final ReentrantReadWriteLock propertyLock = new ReentrantReadWriteLock(); - @Inject - private Gson gson; + /** + * The property attributes for this configuration. + */ + private Map<String, Map<String, String>> propertiesAttributes; + + private Map<PropertyInfo.PropertyType, Set<String>> propertiesTypes; + + private final ClusterDAO clusterDAO; + + private final Gson gson; @Inject private ServiceConfigDAO serviceConfigDAO; - @Inject - private AmbariEventPublisher eventPublisher; + private final AmbariEventPublisher eventPublisher; @AssistedInject - public ConfigImpl(@Assisted Cluster cluster, @Assisted String type, @Assisted Map<String, String> properties, - @Assisted Map<String, Map<String, String>> propertiesAttributes, Injector injector) { + ConfigImpl(@Assisted Cluster cluster, @Assisted("type") String type, + @Assisted("tag") @Nullable String tag, + @Assisted Map<String, String> properties, + @Assisted @Nullable Map<String, Map<String, String>> propertiesAttributes, ClusterDAO clusterDAO, + Gson gson, AmbariEventPublisher eventPublisher) { + this.cluster = cluster; this.type = type; this.properties = properties; - this.propertiesAttributes = propertiesAttributes; + + // only set this if it's non-null + this.propertiesAttributes = null == propertiesAttributes ? null + : new HashMap<>(propertiesAttributes); + + this.clusterDAO = clusterDAO; + this.gson = gson; + this.eventPublisher = eventPublisher; + version = cluster.getNextConfigVersion(type); + + // tag is nullable from factory but not in the DB, so ensure we generate something + tag = StringUtils.isBlank(tag) ? GENERATED_TAG_PREFIX + version : tag; + this.tag = tag; + + ClusterEntity clusterEntity = clusterDAO.findById(cluster.getClusterId()); + + ClusterConfigEntity entity = new ClusterConfigEntity(); + entity.setClusterEntity(clusterEntity); + entity.setClusterId(cluster.getClusterId()); + entity.setType(type); + entity.setVersion(version); + entity.setTag(this.tag); + entity.setTimestamp(System.currentTimeMillis()); + entity.setStack(clusterEntity.getDesiredStack()); + entity.setData(gson.toJson(properties)); + + if (null != propertiesAttributes) { + entity.setAttributes(gson.toJson(propertiesAttributes)); + } // when creating a brand new config without a backing entity, use the // cluster's desired stack as the config's stack stackId = cluster.getDesiredStackVersion(); - - injector.injectMembers(this); propertiesTypes = cluster.getConfigPropertiesTypes(type); - } + persist(entity); + configId = entity.getConfigId(); + } @AssistedInject - public ConfigImpl(@Assisted Cluster cluster, @Assisted ClusterConfigEntity entity, Injector injector) { + ConfigImpl(@Assisted Cluster cluster, @Assisted ClusterConfigEntity entity, + ClusterDAO clusterDAO, Gson gson, AmbariEventPublisher eventPublisher) { this.cluster = cluster; + this.clusterDAO = clusterDAO; + this.gson = gson; + this.eventPublisher = eventPublisher; + configId = entity.getConfigId(); + type = entity.getType(); tag = entity.getTag(); version = entity.getVersion(); @@ -103,16 +154,69 @@ public class ConfigImpl implements Config { // when using an existing entity, use the actual value of the entity's stack stackId = new StackId(entity.getStack()); - this.entity = entity; - injector.injectMembers(this); propertiesTypes = cluster.getConfigPropertiesTypes(type); + + // incur the hit on deserialization since this business object is stored locally + try { + Map<String, String> deserializedProperties = gson.<Map<String, String>> fromJson( + entity.getData(), Map.class); + + if (null == deserializedProperties) { + deserializedProperties = new HashMap<>(); + } + + properties = deserializedProperties; + } catch (JsonSyntaxException e) { + LOG.error("Malformed configuration JSON stored in the database for {}/{}", entity.getType(), + entity.getTag()); + } + + // incur the hit on deserialization since this business object is stored locally + try { + Map<String, Map<String, String>> deserializedAttributes = gson.<Map<String, Map<String, String>>> fromJson( + entity.getAttributes(), Map.class); + + if (null != deserializedAttributes) { + propertiesAttributes = new HashMap<>(deserializedAttributes); + } + } catch (JsonSyntaxException e) { + LOG.error("Malformed configuration attribute JSON stored in the database for {}/{}", + entity.getType(), entity.getTag()); + } } /** - * Constructor for clients not using factory. + * Constructor. This will create an instance suitable only for + * representation/serialization as it is incomplete. + * + * @param type + * @param tag + * @param properties + * @param propertiesAttributes + * @param clusterDAO + * @param gson + * @param eventPublisher */ - public ConfigImpl(String type) { + @AssistedInject + ConfigImpl(@Assisted("type") String type, + @Assisted("tag") @Nullable String tag, + @Assisted Map<String, String> properties, + @Assisted @Nullable Map<String, Map<String, String>> propertiesAttributes, ClusterDAO clusterDAO, + Gson gson, AmbariEventPublisher eventPublisher) { + + this.tag = tag; this.type = type; + this.properties = new HashMap<>(properties); + this.propertiesAttributes = null == propertiesAttributes ? null + : new HashMap<>(propertiesAttributes); + this.clusterDAO = clusterDAO; + this.gson = gson; + this.eventPublisher = eventPublisher; + + cluster = null; + configId = 0; + version = 0L; + stackId = null; } /** @@ -120,240 +224,124 @@ public class ConfigImpl implements Config { */ @Override public StackId getStackId() { - readWriteLock.readLock().lock(); - try { - return stackId; - } finally { - readWriteLock.readLock().unlock(); - } - + return stackId; } @Override public Map<PropertyInfo.PropertyType, Set<String>> getPropertiesTypes() { - readWriteLock.readLock().lock(); - try { - return propertiesTypes; - } finally { - readWriteLock.readLock().unlock(); - } + return propertiesTypes; } @Override public void setPropertiesTypes(Map<PropertyInfo.PropertyType, Set<String>> propertiesTypes) { - readWriteLock.writeLock().lock(); - try { - this.propertiesTypes = propertiesTypes; - } finally { - readWriteLock.writeLock().unlock(); - } - } - - @Override - public void setStackId(StackId stackId) { - readWriteLock.writeLock().lock(); - try { - this.stackId = stackId; - } finally { - readWriteLock.writeLock().unlock(); - } - + this.propertiesTypes = propertiesTypes; } @Override public String getType() { - readWriteLock.readLock().lock(); - try { - return type; - } finally { - readWriteLock.readLock().unlock(); - } - + return type; } @Override public String getTag() { - if (tag == null) { - readWriteLock.writeLock().lock(); - try { - if (tag == null) { - tag = GENERATED_TAG_PREFIX + getVersion(); - } - } finally { - readWriteLock.writeLock().unlock(); - } - } - - readWriteLock.readLock().lock(); - try { - - return tag; - } finally { - readWriteLock.readLock().unlock(); - } - + return tag; } @Override public Long getVersion() { - if (version == null && cluster != null) { - readWriteLock.writeLock().lock(); - try { - if (version == null) { - version = cluster.getNextConfigVersion(type); //pure DB calculation call, no cluster locking required - } - } finally { - readWriteLock.writeLock().unlock(); - } - } - - readWriteLock.readLock().lock(); - try { - return version; - } finally { - readWriteLock.readLock().unlock(); - } - + return version; } @Override public Map<String, String> getProperties() { - if (null != entity && null == properties) { - readWriteLock.writeLock().lock(); - try { - if (properties == null) { - try { - properties = gson.<Map<String, String>>fromJson(entity.getData(), Map.class); - } catch (JsonSyntaxException e){ - String msg = String.format( - "Malformed JSON stored in the database for %s configuration record with config_id %d", - entity.getType(), entity.getConfigId()); - LOG.error(msg); - throw new JsonSyntaxException(msg, e); - } - } - } finally { - readWriteLock.writeLock().unlock(); - } - } - - readWriteLock.readLock().lock(); + propertyLock.readLock().lock(); try { - return null == properties ? new HashMap<String, String>() - : new HashMap<String, String>(properties); + return properties == null ? new HashMap<String, String>() : new HashMap<>(properties); } finally { - readWriteLock.readLock().unlock(); + propertyLock.readLock().unlock(); } - } @Override public Map<String, Map<String, String>> getPropertiesAttributes() { - if (null != entity && null == propertiesAttributes) { - readWriteLock.writeLock().lock(); - try { - if (propertiesAttributes == null) { - propertiesAttributes = gson.<Map<String, Map<String, String>>>fromJson(entity.getAttributes(), Map.class); - } - } finally { - readWriteLock.writeLock().unlock(); - } - } - - readWriteLock.readLock().lock(); - try { - return null == propertiesAttributes ? null : new HashMap<String, Map<String, String>>(propertiesAttributes); - } finally { - readWriteLock.readLock().unlock(); - } - - } - - @Override - public void setTag(String tag) { - readWriteLock.writeLock().lock(); - try { - this.tag = tag; - } finally { - readWriteLock.writeLock().unlock(); - } - - } - - @Override - public void setVersion(Long version) { - readWriteLock.writeLock().lock(); - try { - this.version = version; - } finally { - readWriteLock.writeLock().unlock(); - } - + return null == propertiesAttributes ? null + : new HashMap<String, Map<String, String>>(propertiesAttributes); } @Override public void setProperties(Map<String, String> properties) { - readWriteLock.writeLock().lock(); + propertyLock.writeLock().lock(); try { this.properties = properties; } finally { - readWriteLock.writeLock().unlock(); + propertyLock.writeLock().unlock(); } - } @Override public void setPropertiesAttributes(Map<String, Map<String, String>> propertiesAttributes) { - readWriteLock.writeLock().lock(); - try { - this.propertiesAttributes = propertiesAttributes; - } finally { - readWriteLock.writeLock().unlock(); - } - + this.propertiesAttributes = propertiesAttributes; } @Override - public void updateProperties(Map<String, String> properties) { - readWriteLock.writeLock().lock(); + public void updateProperties(Map<String, String> propertiesToUpdate) { + propertyLock.writeLock().lock(); try { - this.properties.putAll(properties); + properties.putAll(propertiesToUpdate); } finally { - readWriteLock.writeLock().unlock(); + propertyLock.writeLock().unlock(); } - } @Override public List<Long> getServiceConfigVersions() { - readWriteLock.readLock().lock(); - try { - if (cluster == null || type == null || version == null) { - return Collections.emptyList(); - } - return serviceConfigDAO.getServiceConfigVersionsByConfig(cluster.getClusterId(), type, version); - } finally { - readWriteLock.readLock().unlock(); - } - + return serviceConfigDAO.getServiceConfigVersionsByConfig(cluster.getClusterId(), type, version); } @Override - public void deleteProperties(List<String> properties) { - readWriteLock.writeLock().lock(); + public void deleteProperties(List<String> propertyKeysToRemove) { + propertyLock.writeLock().lock(); try { - for (String key : properties) { - this.properties.remove(key); - } + Set<String> keySet = properties.keySet(); + keySet.removeAll(propertyKeysToRemove); } finally { - readWriteLock.writeLock().unlock(); + propertyLock.writeLock().unlock(); } + } + /** + * Persist the entity and update the internal state relationships once the + * transaction has been committed. + */ + private void persist(ClusterConfigEntity entity) { + persistEntitiesInTransaction(entity); + + // ensure that the in-memory state of the cluster is kept consistent + cluster.addConfig(this); + + // re-load the entity associations for the cluster + cluster.refresh(); + + // broadcast the change event for the configuration + ClusterConfigChangedEvent event = new ClusterConfigChangedEvent(cluster.getClusterName(), + getType(), getTag(), getVersion()); + + eventPublisher.publish(event); } - @Override - public void persist() { - persist(true); + /** + * Persist the cluster and configuration entities in their own transaction. + */ + @Transactional + private void persistEntitiesInTransaction(ClusterConfigEntity entity) { + ClusterEntity clusterEntity = entity.getClusterEntity(); + + clusterDAO.createConfig(entity); + clusterEntity.getClusterConfigEntities().add(entity); + + // save the entity, forcing a flush to ensure the refresh picks up the + // newest data + clusterDAO.merge(clusterEntity, true); } /** @@ -361,69 +349,29 @@ public class ConfigImpl implements Config { */ @Override @Transactional - public void persist(boolean newConfig) { - readWriteLock.writeLock().lock(); - try { - ClusterEntity clusterEntity = clusterDAO.findById(cluster.getClusterId()); - - if (newConfig) { - ClusterConfigEntity entity = new ClusterConfigEntity(); - entity.setClusterEntity(clusterEntity); - entity.setClusterId(cluster.getClusterId()); - entity.setType(getType()); - entity.setVersion(getVersion()); - entity.setTag(getTag()); - entity.setTimestamp(new Date().getTime()); - entity.setStack(clusterEntity.getDesiredStack()); - entity.setData(gson.toJson(getProperties())); - - if (null != getPropertiesAttributes()) { - entity.setAttributes(gson.toJson(getPropertiesAttributes())); - } - - clusterDAO.createConfig(entity); - clusterEntity.getClusterConfigEntities().add(entity); - - // save the entity, forcing a flush to ensure the refresh picks up the - // newest data - clusterDAO.merge(clusterEntity, true); - } else { - // only supporting changes to the properties - ClusterConfigEntity entity = null; - - // find the existing configuration to update - for (ClusterConfigEntity cfe : clusterEntity.getClusterConfigEntities()) { - if (getTag().equals(cfe.getTag()) && getType().equals(cfe.getType()) - && getVersion().equals(cfe.getVersion())) { - entity = cfe; - break; - } - } - - // if the configuration was found, then update it - if (null != entity) { - LOG.debug( - "Updating {} version {} with new configurations; a new version will not be created", - getType(), getVersion()); - - entity.setData(gson.toJson(getProperties())); - - // save the entity, forcing a flush to ensure the refresh picks up the - // newest data - clusterDAO.merge(clusterEntity, true); - } - } - } finally { - readWriteLock.writeLock().unlock(); - } + public void save() { + ClusterConfigEntity entity = clusterDAO.findConfig(configId); + ClusterEntity clusterEntity = clusterDAO.findById(entity.getClusterId()); - // re-load the entity associations for the cluster - cluster.refresh(); + // if the configuration was found, then update it + if (null != entity) { + LOG.debug("Updating {} version {} with new configurations; a new version will not be created", + getType(), getVersion()); - // broadcast the change event for the configuration - ClusterConfigChangedEvent event = new ClusterConfigChangedEvent(cluster.getClusterName(), - getType(), getTag(), getVersion()); + entity.setData(gson.toJson(getProperties())); + + // save the entity, forcing a flush to ensure the refresh picks up the + // newest data + clusterDAO.merge(clusterEntity, true); + + // re-load the entity associations for the cluster + cluster.refresh(); + + // broadcast the change event for the configuration + ClusterConfigChangedEvent event = new ClusterConfigChangedEvent(cluster.getClusterName(), + getType(), getTag(), getVersion()); eventPublisher.publish(event); + } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java index 9917720..9a2fc88 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -44,6 +45,7 @@ import org.apache.ambari.server.orm.entities.HostEntity; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; +import org.apache.ambari.server.state.ConfigFactory; import org.apache.ambari.server.state.Host; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -78,7 +80,10 @@ public class ConfigGroupImpl implements ConfigGroup { @Inject private ClusterDAO clusterDAO; @Inject - Clusters clusters; + private Clusters clusters; + + @Inject + private ConfigFactory configFactory; @AssistedInject public ConfigGroupImpl(@Assisted("cluster") Cluster cluster, @@ -398,35 +403,24 @@ public class ConfigGroupImpl implements ConfigGroup { } if (configurations != null && !configurations.isEmpty()) { - for (Config config : configurations.values()) { + for (Entry<String, Config> entry : configurations.entrySet()) { + Config config = entry.getValue(); ClusterConfigEntity clusterConfigEntity = clusterDAO.findConfig (cluster.getClusterId(), config.getType(), config.getTag()); if (clusterConfigEntity == null) { - config.setVersion(cluster.getNextConfigVersion(config.getType())); - config.setStackId(cluster.getDesiredStackVersion()); - // Create configuration - clusterConfigEntity = new ClusterConfigEntity(); - clusterConfigEntity.setClusterId(clusterEntity.getClusterId()); - clusterConfigEntity.setClusterEntity(clusterEntity); - clusterConfigEntity.setStack(clusterEntity.getDesiredStack()); - clusterConfigEntity.setType(config.getType()); - clusterConfigEntity.setVersion(config.getVersion()); - clusterConfigEntity.setTag(config.getTag()); - clusterConfigEntity.setData(gson.toJson(config.getProperties())); - if (null != config.getPropertiesAttributes()) { - clusterConfigEntity.setAttributes(gson.toJson(config.getPropertiesAttributes())); - } - clusterConfigEntity.setTimestamp(System.currentTimeMillis()); - clusterDAO.createConfig(clusterConfigEntity); - clusterEntity.getClusterConfigEntities().add(clusterConfigEntity); - cluster.addConfig(config); - clusterDAO.merge(clusterEntity); - cluster.refresh(); + config = configFactory.createNew(cluster, config.getType(), config.getTag(), + config.getProperties(), config.getPropertiesAttributes()); + + entry.setValue(config); + + clusterConfigEntity = clusterDAO.findConfig(cluster.getClusterId(), config.getType(), + config.getTag()); } ConfigGroupConfigMappingEntity configMappingEntity = new ConfigGroupConfigMappingEntity(); + configMappingEntity.setTimestamp(System.currentTimeMillis()); configMappingEntity.setClusterId(clusterEntity.getClusterId()); configMappingEntity.setClusterConfigEntity(clusterConfigEntity); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java index 83f8470..bb6be30 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java @@ -67,7 +67,7 @@ import org.apache.ambari.server.security.authorization.AuthorizationException; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; -import org.apache.ambari.server.state.ConfigImpl; +import org.apache.ambari.server.state.ConfigFactory; import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.SecurityType; @@ -91,8 +91,13 @@ public class AmbariContext { @Inject private PersistedState persistedState; + /** + * Used for creating read-only instances of existing {@link Config} in order + * to send them to the {@link ConfigGroupResourceProvider} to create + * {@link ConfigGroup}s. + */ @Inject - private org.apache.ambari.server.configuration.Configuration configs; + ConfigFactory configFactory; private static AmbariManagementController controller; private static ClusterController clusterController; @@ -474,11 +479,13 @@ public class AmbariContext { SortedSet<DesiredConfig> desiredConfigsOrderedByVersion = new TreeSet<>(new Comparator<DesiredConfig>() { @Override public int compare(DesiredConfig o1, DesiredConfig o2) { - if (o1.getVersion() < o2.getVersion()) + if (o1.getVersion() < o2.getVersion()) { return -1; + } - if (o1.getVersion() > o2.getVersion()) + if (o1.getVersion() > o2.getVersion()) { return 1; + } return 0; } @@ -489,9 +496,9 @@ public class AmbariContext { int tagMatchState = 0; // 0 -> INITIAL -> tagMatchState = 1 -> TOPLOGY_RESOLVED -> tagMatchState = 2 for (DesiredConfig config: desiredConfigsOrderedByVersion) { - if (config.getTag().equals(TopologyManager.INITIAL_CONFIG_TAG) && tagMatchState == 0) + if (config.getTag().equals(TopologyManager.INITIAL_CONFIG_TAG) && tagMatchState == 0) { tagMatchState = 1; - else if (config.getTag().equals(TopologyManager.TOPOLOGY_RESOLVED_TAG) && tagMatchState == 1) { + } else if (config.getTag().equals(TopologyManager.TOPOLOGY_RESOLVED_TAG) && tagMatchState == 1) { tagMatchState = 2; break; } @@ -605,9 +612,7 @@ public class AmbariContext { for (Map.Entry<String, Map<String, String>> entry : userProvidedGroupProperties.entrySet()) { String type = entry.getKey(); String service = stack.getServiceForConfigType(type); - Config config = new ConfigImpl(type); - config.setTag(groupName); - config.setProperties(entry.getValue()); + Config config = configFactory.createReadOnly(type, groupName, entry.getValue(), null); //todo: attributes Map<String, Config> serviceConfigs = groupConfigs.get(service); if (serviceConfigs == null) { http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java index 6a8057c..4c1ef5a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/update/HostUpdateHelper.java @@ -53,8 +53,8 @@ import org.apache.ambari.server.orm.entities.TopologyRequestEntity; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; +import org.apache.ambari.server.state.ConfigFactory; import org.apache.ambari.server.state.ConfigHelper; -import org.apache.ambari.server.state.ConfigImpl; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.utils.EventBusSynchronizer; import org.apache.commons.lang.StringUtils; @@ -234,12 +234,12 @@ public class HostUpdateHelper { boolean configUpdated; // going through all cluster configs and update property values + ConfigFactory configFactory = injector.getInstance(ConfigFactory.class); for (ClusterConfigEntity clusterConfigEntity : clusterConfigEntities) { - ConfigImpl config = new ConfigImpl(cluster, clusterConfigEntity, injector); + Config config = configFactory.createExisting(cluster, clusterConfigEntity); configUpdated = false; for (Map.Entry<String,String> property : config.getProperties().entrySet()) { - updatedPropertyValue = replaceHosts(property.getValue(), currentHostNames, hostMapping); if (updatedPropertyValue != null) { @@ -249,8 +249,9 @@ public class HostUpdateHelper { configUpdated = true; } } + if (configUpdated) { - config.persist(false); + config.save(); } } } @@ -317,6 +318,7 @@ public class HostUpdateHelper { * */ public class StringComparator implements Comparator<String> { + @Override public int compare(String s1, String s2) { return s2.length() - s1.length(); } http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java index ffca51d..62ce93b 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/ExecutionCommandWrapperTest.java @@ -36,7 +36,6 @@ import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.InMemoryDefaultTestModule; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; -import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.ConfigFactory; import org.apache.ambari.server.state.ConfigHelper; import org.apache.ambari.server.state.StackId; @@ -128,24 +127,16 @@ public class ExecutionCommandWrapperTest { CONFIG_ATTRIBUTES = new HashMap<String, Map<String,String>>(); //Cluster level global config - Config globalConfig = configFactory.createNew(cluster1, GLOBAL_CONFIG, GLOBAL_CLUSTER, CONFIG_ATTRIBUTES); - globalConfig.setTag(CLUSTER_VERSION_TAG); - cluster1.addConfig(globalConfig); + configFactory.createNew(cluster1, GLOBAL_CONFIG, CLUSTER_VERSION_TAG, GLOBAL_CLUSTER, CONFIG_ATTRIBUTES); //Cluster level service config - Config serviceSiteConfigCluster = configFactory.createNew(cluster1, SERVICE_SITE_CONFIG, SERVICE_SITE_CLUSTER, CONFIG_ATTRIBUTES); - serviceSiteConfigCluster.setTag(CLUSTER_VERSION_TAG); - cluster1.addConfig(serviceSiteConfigCluster); + configFactory.createNew(cluster1, SERVICE_SITE_CONFIG, CLUSTER_VERSION_TAG, SERVICE_SITE_CLUSTER, CONFIG_ATTRIBUTES); //Service level service config - Config serviceSiteConfigService = configFactory.createNew(cluster1, SERVICE_SITE_CONFIG, SERVICE_SITE_SERVICE, CONFIG_ATTRIBUTES); - serviceSiteConfigService.setTag(SERVICE_VERSION_TAG); - cluster1.addConfig(serviceSiteConfigService); + configFactory.createNew(cluster1, SERVICE_SITE_CONFIG, SERVICE_VERSION_TAG, SERVICE_SITE_SERVICE, CONFIG_ATTRIBUTES); //Host level service config - Config serviceSiteConfigHost = configFactory.createNew(cluster1, SERVICE_SITE_CONFIG, SERVICE_SITE_HOST, CONFIG_ATTRIBUTES); - serviceSiteConfigHost.setTag(HOST_VERSION_TAG); - cluster1.addConfig(serviceSiteConfigHost); + configFactory.createNew(cluster1, SERVICE_SITE_CONFIG, HOST_VERSION_TAG, SERVICE_SITE_HOST, CONFIG_ATTRIBUTES); ActionDBAccessor db = injector.getInstance(ActionDBAccessorImpl.class); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java index 90a4421..246c8b3 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionSchedulerThreading.java @@ -34,8 +34,8 @@ import org.apache.ambari.server.orm.OrmTestHelper; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; +import org.apache.ambari.server.state.ConfigFactory; import org.apache.ambari.server.state.ConfigHelper; -import org.apache.ambari.server.state.ConfigImpl; import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.StackId; import org.junit.After; @@ -103,15 +103,11 @@ public class TestActionSchedulerThreading { Map<String, String> properties = new HashMap<String, String>(); Map<String, Map<String, String>> propertiesAttributes = new HashMap<String, Map<String, String>>(); + ConfigFactory configFactory = injector.getInstance(ConfigFactory.class); + // foo-type for v1 on current stack properties.put("foo-property-1", "foo-value-1"); - Config c1 = new ConfigImpl(cluster, "foo-type", properties, propertiesAttributes, injector); - c1.setTag("version-1"); - c1.setStackId(stackId); - c1.setVersion(1L); - - cluster.addConfig(c1); - c1.persist(); + Config c1 = configFactory.createNew(cluster, "foo-type", "version-1", properties, propertiesAttributes); // make v1 "current" cluster.addDesiredConfig("admin", Sets.newHashSet(c1), "note-1"); @@ -122,12 +118,7 @@ public class TestActionSchedulerThreading { // save v2 // foo-type for v2 on new stack properties.put("foo-property-2", "foo-value-2"); - Config c2 = new ConfigImpl(cluster, "foo-type", properties, propertiesAttributes, injector); - c2.setTag("version-2"); - c2.setStackId(newStackId); - c2.setVersion(2L); - cluster.addConfig(c2); - c2.persist(); + Config c2 = configFactory.createNew(cluster, "foo-type", "version-2", properties, propertiesAttributes); // make v2 "current" cluster.addDesiredConfig("admin", Sets.newHashSet(c2), "note-2"); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatTestHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatTestHelper.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatTestHelper.java index 43503fa..fc2bca5 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatTestHelper.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/HeartbeatTestHelper.java @@ -193,11 +193,7 @@ public class HeartbeatTestHelper { cluster.setCurrentStackVersion(stackId); ConfigFactory cf = injector.getInstance(ConfigFactory.class); - Config config = cf.createNew(cluster, "cluster-env", configProperties, new HashMap<String, Map<String, String>>()); - config.setTag("version1"); - config.persist(); - - cluster.addConfig(config); + Config config = cf.createNew(cluster, "cluster-env", "version1", configProperties, new HashMap<String, Map<String, String>>()); cluster.addDesiredConfig("user", Collections.singleton(config)); helper.getOrCreateRepositoryVersion(stackId, stackId.getStackVersion()); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java b/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java index 76ab45c..68e9993 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatMonitor.java @@ -159,10 +159,8 @@ public class TestHeartbeatMonitor { }}; ConfigFactory configFactory = injector.getInstance(ConfigFactory.class); - Config config = configFactory.createNew(cluster, "hadoop-env", + Config config = configFactory.createNew(cluster, "hadoop-env", "version1", new HashMap<String,String>() {{ put("a", "b"); }}, new HashMap<String, Map<String,String>>()); - config.setTag("version1"); - cluster.addConfig(config); cluster.addDesiredConfig("_test", Collections.singleton(config)); @@ -243,18 +241,15 @@ public class TestHeartbeatMonitor { }}; ConfigFactory configFactory = injector.getInstance(ConfigFactory.class); - Config hadoopEnvConfig = configFactory.createNew(cluster, "hadoop-env", + Config hadoopEnvConfig = configFactory.createNew(cluster, "hadoop-env", "version1", new HashMap<String, String>() {{ put("a", "b"); }}, new HashMap<String, Map<String,String>>()); - Config hbaseEnvConfig = configFactory.createNew(cluster, "hbase-env", + Config hbaseEnvConfig = configFactory.createNew(cluster, "hbase-env", "version1", new HashMap<String, String>() {{ put("a", "b"); }}, new HashMap<String, Map<String,String>>()); - hadoopEnvConfig.setTag("version1"); - cluster.addConfig(hadoopEnvConfig); - hbaseEnvConfig.setTag("version1"); - cluster.addConfig(hbaseEnvConfig); + cluster.addDesiredConfig("_test", Collections.singleton(hadoopEnvConfig)); http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/test/java/org/apache/ambari/server/configuration/RecoveryConfigHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/configuration/RecoveryConfigHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/configuration/RecoveryConfigHelperTest.java index 6533e1c..6640837 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/configuration/RecoveryConfigHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/configuration/RecoveryConfigHelperTest.java @@ -218,7 +218,7 @@ public class RecoveryConfigHelperTest { config.updateProperties(new HashMap<String, String>() {{ put(RecoveryConfigHelper.RECOVERY_ENABLED_KEY, "false"); }}); - config.persist(false); + config.save(); // Recovery config should be stale because of the above change. boolean isConfigStale = recoveryConfigHelper.isConfigStale(cluster.getClusterName(), DummyHostname1, http://git-wip-us.apache.org/repos/asf/ambari/blob/a6639a7c/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java index e54a117..2507a46 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java @@ -87,8 +87,8 @@ import org.apache.ambari.server.security.ldap.LdapBatchDto; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.ComponentInfo; +import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.ConfigHelper; -import org.apache.ambari.server.state.ConfigImpl; import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.MaintenanceState; @@ -610,6 +610,7 @@ public class AmbariManagementControllerImplTest { Cluster cluster = createNiceMock(Cluster.class); ActionManager actionManager = createNiceMock(ActionManager.class); ClusterRequest clusterRequest = createNiceMock(ClusterRequest.class); + Config config = createNiceMock(Config.class); // requests Set<ClusterRequest> setRequests = Collections.singleton(clusterRequest); @@ -632,18 +633,11 @@ public class AmbariManagementControllerImplTest { expect(clusters.getClusterById(1L)).andReturn(cluster).anyTimes(); expect(cluster.getClusterName()).andReturn("clusterOld").anyTimes(); expect(cluster.getConfigPropertiesTypes(anyObject(String.class))).andReturn(Maps.<PropertyInfo.PropertyType, Set<String>>newHashMap()).anyTimes(); - expect(cluster.getDesiredConfigByType(anyObject(String.class))).andReturn(new ConfigImpl("config-type") { - @Override - public Map<String, Map<String, String>> getPropertiesAttributes() { - return Maps.newHashMap(); - } - - @Override - public Map<String, String> getProperties() { - return configReqProps; - } - }).anyTimes(); + expect(config.getType()).andReturn("config-type").anyTimes(); + expect(config.getProperties()).andReturn(configReqProps).anyTimes(); + expect(config.getPropertiesAttributes()).andReturn(new HashMap<String,Map<String,String>>()).anyTimes(); + expect(cluster.getDesiredConfigByType(anyObject(String.class))).andReturn(config).anyTimes(); cluster.addSessionAttributes(anyObject(Map.class)); expectLastCall().once(); @@ -652,7 +646,7 @@ public class AmbariManagementControllerImplTest { expectLastCall(); // replay mocks - replay(actionManager, cluster, clusters, injector, clusterRequest, sessionManager); + replay(actionManager, cluster, clusters, config, injector, clusterRequest, sessionManager); // test AmbariManagementController controller = new AmbariManagementControllerImpl(actionManager, clusters, injector); @@ -660,7 +654,7 @@ public class AmbariManagementControllerImplTest { // assert and verify assertSame(controller, controllerCapture.getValue()); - verify(actionManager, cluster, clusters, injector, clusterRequest, sessionManager); + verify(actionManager, cluster, clusters, config, injector, clusterRequest, sessionManager); } /**