AMBARI-18933 - Remove Unnecessary Locks Inside Of ConfigGroup 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/ab1c1001 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ab1c1001 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ab1c1001 Branch: refs/heads/branch-feature-AMBARI-18456 Commit: ab1c1001688b333817ac20ec5c20b635f566c353 Parents: 0a0e9a5 Author: Jonathan Hurley <jhur...@hortonworks.com> Authored: Fri Nov 18 12:48:52 2016 -0500 Committer: Jonathan Hurley <jhur...@hortonworks.com> Committed: Fri Nov 18 18:36:41 2016 -0500 ---------------------------------------------------------------------- .../internal/ConfigGroupResourceProvider.java | 49 +- .../apache/ambari/server/state/ConfigImpl.java | 14 +- .../server/state/cluster/ClusterImpl.java | 6 +- .../server/state/configgroup/ConfigGroup.java | 33 +- .../state/configgroup/ConfigGroupFactory.java | 34 +- .../state/configgroup/ConfigGroupImpl.java | 583 +++++++++---------- .../ambari/server/topology/AmbariContext.java | 2 - .../AmbariManagementControllerTest.java | 2 - .../ambari/server/state/ConfigGroupTest.java | 18 +- .../ambari/server/state/ConfigHelperTest.java | 1 - .../server/state/cluster/ClusterTest.java | 7 - .../svccomphost/ServiceComponentHostTest.java | 3 - .../server/topology/AmbariContextTest.java | 1 - 13 files changed, 342 insertions(+), 411 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/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 b957f0a..2373068 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 @@ -17,7 +17,16 @@ */ package org.apache.ambari.server.controller.internal; -import com.google.inject.Inject; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.ClusterNotFoundException; import org.apache.ambari.server.ConfigGroupNotFoundException; @@ -49,7 +58,6 @@ 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; import org.apache.ambari.server.state.configgroup.ConfigGroupFactory; @@ -57,15 +65,7 @@ import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.EnumSet; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; +import com.google.inject.Inject; @StaticallyInject public class ConfigGroupResourceProvider extends @@ -101,7 +101,7 @@ public class ConfigGroupResourceProvider extends @Inject private static HostDAO hostDAO; - + /** * Used for creating {@link Config} instances to return in the REST response. */ @@ -575,22 +575,19 @@ public class ConfigGroupResourceProvider extends } } + configLogger.info("User {} is creating new configuration group {} for tag {} in cluster {}", + getManagementController().getAuthName(), request.getGroupName(), request.getTag(), + cluster.getClusterName()); + ConfigGroup configGroup = configGroupFactory.createNew(cluster, request.getGroupName(), request.getTag(), request.getDescription(), request.getConfigs(), hosts); - verifyConfigs(configGroup.getConfigurations(), cluster.getClusterName()); configGroup.setServiceName(serviceName); - // Persist before add, since id is auto-generated - configLogger.info("Persisting new Config group" - + ", clusterName = " + cluster.getClusterName() - + ", name = " + configGroup.getName() - + ", tag = " + configGroup.getTag() - + ", user = " + getManagementController().getAuthName()); + verifyConfigs(configGroup.getConfigurations(), cluster.getClusterName()); - configGroup.persist(); cluster.addConfigGroup(configGroup); if (serviceName != null) { cluster.createServiceConfigVersion(serviceName, getManagementController().getAuthName(), @@ -641,6 +638,11 @@ public class ConfigGroupResourceProvider extends + ", clusterName = " + request.getClusterName() + ", groupId = " + request.getId()); } + + configLogger.info("User {} is updating configuration group {} for tag {} in cluster {}", + getManagementController().getAuthName(), request.getGroupName(), request.getTag(), + cluster.getClusterName()); + String serviceName = configGroup.getServiceName(); String requestServiceName = cluster.getServiceForConfigTypes(request.getConfigs().keySet()); if (StringUtils.isEmpty(serviceName) && StringUtils.isEmpty(requestServiceName)) { @@ -689,13 +691,6 @@ public class ConfigGroupResourceProvider extends configGroup.setDescription(request.getDescription()); configGroup.setTag(request.getTag()); - configLogger.info("Persisting updated Config group" - + ", clusterName = " + configGroup.getClusterName() - + ", id = " + configGroup.getId() - + ", tag = " + configGroup.getTag() - + ", user = " + getManagementController().getAuthName()); - - configGroup.persist(); if (serviceName != null) { cluster.createServiceConfigVersion(serviceName, getManagementController().getAuthName(), request.getServiceConfigVersionNote(), configGroup); http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/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 e68839f..052ee28 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 @@ -23,12 +23,13 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.concurrent.locks.ReadWriteLock; import javax.annotation.Nullable; import org.apache.ambari.server.events.ClusterConfigChangedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; +import org.apache.ambari.server.logging.LockFactory; import org.apache.ambari.server.orm.dao.ClusterDAO; import org.apache.ambari.server.orm.dao.ServiceConfigDAO; import org.apache.ambari.server.orm.entities.ClusterConfigEntity; @@ -71,7 +72,7 @@ public class ConfigImpl implements Config { * * @see #properties */ - private final ReentrantReadWriteLock propertyLock = new ReentrantReadWriteLock(); + private final ReadWriteLock propertyLock; /** * The property attributes for this configuration. @@ -94,7 +95,9 @@ public class ConfigImpl implements Config { @Assisted("tag") @Nullable String tag, @Assisted Map<String, String> properties, @Assisted @Nullable Map<String, Map<String, String>> propertiesAttributes, ClusterDAO clusterDAO, - Gson gson, AmbariEventPublisher eventPublisher) { + Gson gson, AmbariEventPublisher eventPublisher, LockFactory lockFactory) { + + propertyLock = lockFactory.newReadWriteLock("configurationPropertyLock"); this.cluster = cluster; this.type = type; @@ -140,7 +143,8 @@ public class ConfigImpl implements Config { @AssistedInject ConfigImpl(@Assisted Cluster cluster, @Assisted ClusterConfigEntity entity, - ClusterDAO clusterDAO, Gson gson, AmbariEventPublisher eventPublisher) { + ClusterDAO clusterDAO, Gson gson, AmbariEventPublisher eventPublisher, + LockFactory lockFactory) { this.cluster = cluster; this.clusterDAO = clusterDAO; this.gson = gson; @@ -333,7 +337,7 @@ public class ConfigImpl implements Config { * Persist the cluster and configuration entities in their own transaction. */ @Transactional - private void persistEntitiesInTransaction(ClusterConfigEntity entity) { + void persistEntitiesInTransaction(ClusterConfigEntity entity) { ClusterEntity clusterEntity = entity.getClusterEntity(); clusterDAO.createConfig(entity); http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java index 8b157c7..6be36dd 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java @@ -326,8 +326,11 @@ public class ClusterImpl implements Cluster { loadStackVersion(); loadServices(); loadServiceHostComponents(); - loadConfigGroups(); + + // cache configurations before loading configuration groups cacheConfigurations(); + loadConfigGroups(); + loadRequestExecutions(); if (desiredStackVersion != null && !StringUtils.isEmpty(desiredStackVersion.getStackName()) && ! @@ -2568,7 +2571,6 @@ public class ClusterImpl implements Cluster { } } configGroup.setHosts(groupDesiredHosts); - configGroup.persist(); } else { throw new IllegalArgumentException("Config group {} doesn't exist"); } http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java index 1b29c9b..5a9c574 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java @@ -18,13 +18,13 @@ package org.apache.ambari.server.state.configgroup; +import java.util.Map; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.controller.ConfigGroupResponse; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.Host; -import java.util.Map; - /** * Configuration group or Config group is a type of Ambari resource that * supports grouping of configuration resources and host resources for a @@ -80,29 +80,20 @@ public interface ConfigGroup { public void setDescription(String description); /** - * List of hosts to which configs are applied + * Gets an unmodifiable list of {@link Host}s. + * * @return */ public Map<Long, Host> getHosts(); /** - * List of @Config objects + * Gets an unmodifiable map of {@link Config}s. + * * @return */ public Map<String, Config> getConfigurations(); /** - * Persist the Config group along with the related host and config mapping - * entities to the persistence store - */ - void persist(); - - /** - * Persist the host mapping entity to the persistence store - */ - void persistHostMapping(); - - /** * Delete config group and the related host and config mapping * entities from the persistence store */ @@ -116,13 +107,6 @@ public interface ConfigGroup { public void addHost(Host host) throws AmbariException; /** - * Add config to the config group - * @param config - * @throws AmbariException - */ - public void addConfiguration(Config config) throws AmbariException; - - /** * Return @ConfigGroupResponse for the config group * * @return @ConfigGroupResponse @@ -131,11 +115,6 @@ public interface ConfigGroup { public ConfigGroupResponse convertToResponse() throws AmbariException; /** - * Refresh Config group and the host and config mappings for the group - */ - public void refresh(); - - /** * Reassign the set of hosts associated with this config group * @param hosts */ http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java index 9abadf3..906d948 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java @@ -17,22 +17,38 @@ */ package org.apache.ambari.server.state.configgroup; -import com.google.inject.assistedinject.Assisted; +import java.util.Map; + import org.apache.ambari.server.orm.entities.ConfigGroupEntity; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.Host; -import org.apache.ambari.server.state.configgroup.ConfigGroup; -import java.util.Map; +import com.google.inject.assistedinject.Assisted; public interface ConfigGroupFactory { - ConfigGroup createNew(@Assisted("cluster") Cluster cluster, - @Assisted("name") String name, - @Assisted("tag") String tag, - @Assisted("description") String description, - @Assisted("configs") Map<String, Config> configs, - @Assisted("hosts") Map<Long, Host> hosts); + /** + * Creates and saves a new {@link ConfigGroup}. + * + * @param cluster + * @param name + * @param tag + * @param description + * @param configs + * @param hosts + * @param serviceName + * @return + */ + ConfigGroup createNew(@Assisted("cluster") Cluster cluster, @Assisted("name") String name, + @Assisted("tag") String tag, @Assisted("description") String description, + @Assisted("configs") Map<String, Config> configs, @Assisted("hosts") Map<Long, Host> hosts); + /** + * Instantiates a {@link ConfigGroup} fron an existing, persisted entity. + * + * @param cluster + * @param entity + * @return + */ ConfigGroup createExisting(Cluster cluster, ConfigGroupEntity entity); } http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/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 9a2fc88..fe1f338 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 @@ -17,19 +17,22 @@ */ package org.apache.ambari.server.state.configgroup; +import java.util.Collection; 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.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.DuplicateResourceException; import org.apache.ambari.server.controller.ConfigGroupResponse; import org.apache.ambari.server.controller.internal.ConfigurationResourceProvider; +import org.apache.ambari.server.logging.LockFactory; import org.apache.ambari.server.orm.dao.ClusterDAO; import org.apache.ambari.server.orm.dao.ConfigGroupConfigMappingDAO; import org.apache.ambari.server.orm.dao.ConfigGroupDAO; @@ -50,212 +53,190 @@ import org.apache.ambari.server.state.Host; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.gson.Gson; -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; public class ConfigGroupImpl implements ConfigGroup { private static final Logger LOG = LoggerFactory.getLogger(ConfigGroupImpl.class); - private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private Cluster cluster; - private ConfigGroupEntity configGroupEntity; - private Map<Long, Host> hosts; - private Map<String, Config> configurations; - private volatile boolean isPersisted = false; - - @Inject - private Gson gson; - @Inject - private ConfigGroupDAO configGroupDAO; - @Inject - private ConfigGroupConfigMappingDAO configGroupConfigMappingDAO; - @Inject - private ConfigGroupHostMappingDAO configGroupHostMappingDAO; - @Inject - private HostDAO hostDAO; - @Inject - private ClusterDAO clusterDAO; - @Inject - private Clusters clusters; - - @Inject - private ConfigFactory configFactory; + private ConcurrentMap<Long, Host> m_hosts; + private ConcurrentMap<String, Config> m_configurations; + private String configGroupName; + private long configGroupId; + + /** + * This lock is required to prevent inconsistencies in internal state between + * {@link #m_hosts} and the entities stored by the {@link ConfigGroupEntity}. + */ + private final ReadWriteLock hostLock; + + /** + * A label for {@link #hostLock} to use with the {@link LockFactory}. + */ + private static final String hostLockLabel = "configurationGroupHostLock"; + + private final ConfigGroupDAO configGroupDAO; + + private final ConfigGroupConfigMappingDAO configGroupConfigMappingDAO; + + private final ConfigGroupHostMappingDAO configGroupHostMappingDAO; + + private final HostDAO hostDAO; + + private final ClusterDAO clusterDAO; + + private final ConfigFactory configFactory; @AssistedInject - public ConfigGroupImpl(@Assisted("cluster") Cluster cluster, - @Assisted("name") String name, - @Assisted("tag") String tag, - @Assisted("description") String description, - @Assisted("configs") Map<String, Config> configs, - @Assisted("hosts") Map<Long, Host> hosts, - Injector injector) { - injector.injectMembers(this); + public ConfigGroupImpl(@Assisted("cluster") Cluster cluster, @Assisted("name") String name, + @Assisted("tag") String tag, @Assisted("description") String description, + @Assisted("configs") Map<String, Config> configurations, + @Assisted("hosts") Map<Long, Host> hosts, Clusters clusters, ConfigFactory configFactory, + ClusterDAO clusterDAO, HostDAO hostDAO, ConfigGroupDAO configGroupDAO, + ConfigGroupConfigMappingDAO configGroupConfigMappingDAO, + ConfigGroupHostMappingDAO configGroupHostMappingDAO, LockFactory lockFactory) { + + this.configFactory = configFactory; + this.clusterDAO = clusterDAO; + this.hostDAO = hostDAO; + this.configGroupDAO = configGroupDAO; + this.configGroupConfigMappingDAO = configGroupConfigMappingDAO; + this.configGroupHostMappingDAO = configGroupHostMappingDAO; + + hostLock = lockFactory.newReadWriteLock(hostLockLabel); + this.cluster = cluster; + configGroupName = name; - configGroupEntity = new ConfigGroupEntity(); + ConfigGroupEntity configGroupEntity = new ConfigGroupEntity(); configGroupEntity.setClusterId(cluster.getClusterId()); configGroupEntity.setGroupName(name); configGroupEntity.setTag(tag); configGroupEntity.setDescription(description); - if (hosts != null) { - this.hosts = hosts; - } else { - this.hosts = new HashMap<Long, Host>(); - } + m_hosts = hosts == null ? new ConcurrentHashMap<Long, Host>() + : new ConcurrentHashMap<>(hosts); - if (configs != null) { - configurations = configs; - } else { - configurations = new HashMap<String, Config>(); - } + m_configurations = configurations == null ? new ConcurrentHashMap<String, Config>() + : new ConcurrentHashMap<>(configurations); + + // save the entity and grab the ID + persist(configGroupEntity); + configGroupId = configGroupEntity.getGroupId(); } @AssistedInject - public ConfigGroupImpl(@Assisted Cluster cluster, - @Assisted ConfigGroupEntity configGroupEntity, - Injector injector) { - injector.injectMembers(this); + public ConfigGroupImpl(@Assisted Cluster cluster, @Assisted ConfigGroupEntity configGroupEntity, + Clusters clusters, ConfigFactory configFactory, + ClusterDAO clusterDAO, HostDAO hostDAO, ConfigGroupDAO configGroupDAO, + ConfigGroupConfigMappingDAO configGroupConfigMappingDAO, + ConfigGroupHostMappingDAO configGroupHostMappingDAO, LockFactory lockFactory) { + + this.configFactory = configFactory; + this.clusterDAO = clusterDAO; + this.hostDAO = hostDAO; + this.configGroupDAO = configGroupDAO; + this.configGroupConfigMappingDAO = configGroupConfigMappingDAO; + this.configGroupHostMappingDAO = configGroupHostMappingDAO; + + hostLock = lockFactory.newReadWriteLock(hostLockLabel); + this.cluster = cluster; + configGroupId = configGroupEntity.getGroupId(); + configGroupName = configGroupEntity.getGroupName(); - this.configGroupEntity = configGroupEntity; - configurations = new HashMap<String, Config>(); - hosts = new HashMap<Long, Host>(); + m_configurations = new ConcurrentHashMap<String, Config>(); + m_hosts = new ConcurrentHashMap<Long, Host>(); // Populate configs - for (ConfigGroupConfigMappingEntity configMappingEntity : configGroupEntity - .getConfigGroupConfigMappingEntities()) { - + for (ConfigGroupConfigMappingEntity configMappingEntity : configGroupEntity.getConfigGroupConfigMappingEntities()) { Config config = cluster.getConfig(configMappingEntity.getConfigType(), configMappingEntity.getVersionTag()); if (config != null) { - configurations.put(config.getType(), config); + m_configurations.put(config.getType(), config); } else { - LOG.warn("Unable to find config mapping for config group" - + ", clusterName = " + cluster.getClusterName() - + ", type = " + configMappingEntity.getConfigType() - + ", tag = " + configMappingEntity.getVersionTag()); + LOG.warn("Unable to find config mapping {}/{} for config group in cluster {}", + configMappingEntity.getConfigType(), configMappingEntity.getVersionTag(), + cluster.getClusterName()); } } // Populate Hosts - for (ConfigGroupHostMappingEntity hostMappingEntity : configGroupEntity - .getConfigGroupHostMappingEntities()) { - + for (ConfigGroupHostMappingEntity hostMappingEntity : configGroupEntity.getConfigGroupHostMappingEntities()) { try { Host host = clusters.getHost(hostMappingEntity.getHostname()); HostEntity hostEntity = hostMappingEntity.getHostEntity(); if (host != null && hostEntity != null) { - hosts.put(hostEntity.getHostId(), host); + m_hosts.put(hostEntity.getHostId(), host); } } catch (AmbariException e) { - String msg = "Host seems to be deleted but Config group mapping still " + - "exists !"; - LOG.warn(msg); - LOG.debug(msg, e); + LOG.warn("Host seems to be deleted but Config group mapping still exists !"); + LOG.debug("Host seems to be deleted but Config group mapping still exists !", e); } } - - isPersisted = true; } @Override public Long getId() { - return configGroupEntity.getGroupId(); + return configGroupId; } @Override public String getName() { - readWriteLock.readLock().lock(); - try { - return configGroupEntity.getGroupName(); - } finally { - readWriteLock.readLock().unlock(); - } + return configGroupName; } @Override public void setName(String name) { - readWriteLock.writeLock().lock(); - try { - configGroupEntity.setGroupName(name); - } finally { - readWriteLock.writeLock().unlock(); - } + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + configGroupEntity.setGroupName(name); + configGroupDAO.merge(configGroupEntity); + configGroupName = name; } @Override public String getClusterName() { - return configGroupEntity.getClusterEntity().getClusterName(); + return cluster.getClusterName(); } @Override public String getTag() { - readWriteLock.readLock().lock(); - try { - return configGroupEntity.getTag(); - } finally { - readWriteLock.readLock().unlock(); - } + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + return configGroupEntity.getTag(); } @Override public void setTag(String tag) { - readWriteLock.writeLock().lock(); - try { - configGroupEntity.setTag(tag); - } finally { - readWriteLock.writeLock().unlock(); - } - + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + configGroupEntity.setTag(tag); + configGroupDAO.merge(configGroupEntity); } @Override public String getDescription() { - readWriteLock.readLock().lock(); - try { - return configGroupEntity.getDescription(); - } finally { - readWriteLock.readLock().unlock(); - } + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + return configGroupEntity.getDescription(); } @Override public void setDescription(String description) { - readWriteLock.writeLock().lock(); - try { - configGroupEntity.setDescription(description); - } finally { - readWriteLock.writeLock().unlock(); - } - + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + configGroupEntity.setDescription(description); + configGroupDAO.merge(configGroupEntity); } @Override public Map<Long, Host> getHosts() { - readWriteLock.readLock().lock(); - try { - return Collections.unmodifiableMap(hosts); - } finally { - readWriteLock.readLock().unlock(); - } + return Collections.unmodifiableMap(m_hosts); } @Override public Map<String, Config> getConfigurations() { - readWriteLock.readLock().lock(); - try { - return Collections.unmodifiableMap(configurations); - } finally { - readWriteLock.readLock().unlock(); - } - + return Collections.unmodifiableMap(m_configurations); } /** @@ -264,13 +245,14 @@ public class ConfigGroupImpl implements ConfigGroup { */ @Override public void setHosts(Map<Long, Host> hosts) { - readWriteLock.writeLock().lock(); + hostLock.writeLock().lock(); try { - this.hosts = hosts; + // persist enitites in a transaction first, then update internal state + replaceHostMappings(hosts); + m_hosts = new ConcurrentHashMap<>(hosts); } finally { - readWriteLock.writeLock().unlock(); + hostLock.writeLock().unlock(); } - } /** @@ -278,115 +260,140 @@ public class ConfigGroupImpl implements ConfigGroup { * @param configs */ @Override - public void setConfigurations(Map<String, Config> configs) { - readWriteLock.writeLock().lock(); - try { - configurations = configs; - } finally { - readWriteLock.writeLock().unlock(); - } - + public void setConfigurations(Map<String, Config> configurations) { + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + ClusterEntity clusterEntity = configGroupEntity.getClusterEntity(); + + // only update the internal state after the configurations have been + // persisted + persistConfigMapping(clusterEntity, configGroupEntity, configurations); + m_configurations = new ConcurrentHashMap<>(configurations); } @Override - @Transactional public void removeHost(Long hostId) throws AmbariException { - readWriteLock.writeLock().lock(); + hostLock.writeLock().lock(); try { - if (hosts.containsKey(hostId)) { - String hostName = hosts.get(hostId).getHostName(); - LOG.info("Removing host from config group, hostid = " + hostId + ", hostname = " + hostName); - hosts.remove(hostId); - try { - ConfigGroupHostMappingEntityPK hostMappingEntityPK = new - ConfigGroupHostMappingEntityPK(); - hostMappingEntityPK.setHostId(hostId); - hostMappingEntityPK.setConfigGroupId(configGroupEntity.getGroupId()); - configGroupHostMappingDAO.removeByPK(hostMappingEntityPK); - } catch (Exception e) { - LOG.error("Failed to delete config group host mapping" - + ", clusterName = " + getClusterName() - + ", id = " + getId() - + ", hostid = " + hostId - + ", hostname = " + hostName, e); - throw new AmbariException(e.getMessage()); - } + Host host = m_hosts.get(hostId); + if (null == host) { + return; } - } finally { - readWriteLock.writeLock().unlock(); - } - } - @Override - public void persist() { - readWriteLock.writeLock().lock(); - try { - if (!isPersisted) { - persistEntities(); - refresh(); - cluster.refresh(); - isPersisted = true; - } else { - saveIfPersisted(); + String hostName = host.getHostName(); + LOG.info("Removing host (id={}, name={}) from config group", host.getHostId(), hostName); + + try { + // remove the entities first, then update internal state + removeConfigGroupHostEntity(host); + m_hosts.remove(hostId); + } catch (Exception e) { + LOG.error("Failed to delete config group host mapping for cluster {} and host {}", + cluster.getClusterName(), hostName, e); + + throw new AmbariException(e.getMessage()); } } finally { - readWriteLock.writeLock().unlock(); + hostLock.writeLock().unlock(); } } /** + * Removes the {@link ConfigGroupHostMappingEntity} for the specified host + * from this configuration group. + * + * @param host + * the host to remove. + */ + @Transactional + void removeConfigGroupHostEntity(Host host) { + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + ConfigGroupHostMappingEntityPK hostMappingEntityPK = new ConfigGroupHostMappingEntityPK(); + hostMappingEntityPK.setHostId(host.getHostId()); + hostMappingEntityPK.setConfigGroupId(configGroupId); + + ConfigGroupHostMappingEntity configGroupHostMapping = configGroupHostMappingDAO.findByPK( + hostMappingEntityPK); + + configGroupHostMappingDAO.remove(configGroupHostMapping); + + configGroupEntity.getConfigGroupHostMappingEntities().remove(configGroupHostMapping); + configGroupEntity = configGroupDAO.merge(getConfigGroupEntity()); + } + + /** + * @param configGroupEntity + */ + private void persist(ConfigGroupEntity configGroupEntity) { + persistEntities(configGroupEntity); + cluster.refresh(); + } + + /** * Persist Config group with host mapping and configurations * * @throws Exception */ @Transactional - void persistEntities() { + void persistEntities(ConfigGroupEntity configGroupEntity) { ClusterEntity clusterEntity = clusterDAO.findById(cluster.getClusterId()); configGroupEntity.setClusterEntity(clusterEntity); configGroupEntity.setTimestamp(System.currentTimeMillis()); configGroupDAO.create(configGroupEntity); - persistConfigMapping(clusterEntity); - persistHostMapping(); - } + configGroupId = configGroupEntity.getGroupId(); - // TODO: Test rollback scenario + persistConfigMapping(clusterEntity, configGroupEntity, m_configurations); + replaceHostMappings(m_hosts); + } /** - * Persist host mapping + * Replaces all existing host mappings with the new collection of hosts. * + * @param the + * new hosts * @throws Exception */ - @Override @Transactional - public void persistHostMapping() { - if (isPersisted) { - // Delete existing mappings and create new ones - configGroupHostMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); - configGroupEntity.setConfigGroupHostMappingEntities(new HashSet<ConfigGroupHostMappingEntity>()); - } + void replaceHostMappings(Map<Long, Host> hosts) { + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + + // Delete existing mappings and create new ones + configGroupHostMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); + configGroupEntity.setConfigGroupHostMappingEntities( + new HashSet<ConfigGroupHostMappingEntity>()); if (hosts != null && !hosts.isEmpty()) { - for (Host host : hosts.values()) { - HostEntity hostEntity = hostDAO.findById(host.getHostId()); - if (hostEntity != null) { - ConfigGroupHostMappingEntity hostMappingEntity = new - ConfigGroupHostMappingEntity(); - hostMappingEntity.setHostId(hostEntity.getHostId()); - hostMappingEntity.setHostEntity(hostEntity); - hostMappingEntity.setConfigGroupEntity(configGroupEntity); - hostMappingEntity.setConfigGroupId(configGroupEntity.getGroupId()); - configGroupEntity.getConfigGroupHostMappingEntities().add - (hostMappingEntity); - configGroupHostMappingDAO.create(hostMappingEntity); - } else { - LOG.warn("Host seems to be deleted, cannot create host to config " + - "group mapping, host = " + host.getHostName()); - } + configGroupEntity = persistHostMapping(hosts.values(), configGroupEntity); + } + } + + /** + * Adds the collection of hosts to the configuration group. + * + * @param hostEntity + * @param configGroupEntity + */ + @Transactional + ConfigGroupEntity persistHostMapping(Collection<Host> hosts, + ConfigGroupEntity configGroupEntity) { + for (Host host : hosts) { + HostEntity hostEntity = hostDAO.findById(host.getHostId()); + if (hostEntity != null) { + ConfigGroupHostMappingEntity hostMappingEntity = new ConfigGroupHostMappingEntity(); + hostMappingEntity.setHostId(hostEntity.getHostId()); + hostMappingEntity.setHostEntity(hostEntity); + hostMappingEntity.setConfigGroupEntity(configGroupEntity); + hostMappingEntity.setConfigGroupId(configGroupEntity.getGroupId()); + configGroupEntity.getConfigGroupHostMappingEntities().add(hostMappingEntity); + configGroupHostMappingDAO.create(hostMappingEntity); + } else { + LOG.warn( + "The host {} has been removed from the cluster and cannot be added to the configuration group {}", + host.getHostName(), configGroupName); } } - // TODO: Make sure this does not throw Nullpointer based on JPA docs - configGroupEntity = configGroupDAO.merge(configGroupEntity); + + return configGroupDAO.merge(configGroupEntity); } /** @@ -396,11 +403,11 @@ public class ConfigGroupImpl implements ConfigGroup { * @throws Exception */ @Transactional - void persistConfigMapping(ClusterEntity clusterEntity) { - if (isPersisted) { - configGroupConfigMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); - configGroupEntity.setConfigGroupConfigMappingEntities(new HashSet<ConfigGroupConfigMappingEntity>()); - } + void persistConfigMapping(ClusterEntity clusterEntity, + ConfigGroupEntity configGroupEntity, Map<String, Config> configurations) { + configGroupConfigMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); + configGroupEntity.setConfigGroupConfigMappingEntities( + new HashSet<ConfigGroupConfigMappingEntity>()); if (configurations != null && !configurations.isEmpty()) { for (Entry<String, Config> entry : configurations.entrySet()) { @@ -437,142 +444,84 @@ public class ConfigGroupImpl implements ConfigGroup { } } - void saveIfPersisted() { - if (isPersisted) { - save(clusterDAO.findById(cluster.getClusterId())); - } - } - - @Transactional - void save(ClusterEntity clusterEntity) { - persistHostMapping(); - persistConfigMapping(clusterEntity); - } - @Override + @Transactional public void delete() { - readWriteLock.writeLock().lock(); - try { - configGroupConfigMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); - configGroupHostMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); - configGroupDAO.removeByPK(configGroupEntity.getGroupId()); - cluster.refresh(); - isPersisted = false; - } finally { - readWriteLock.writeLock().unlock(); - } + configGroupConfigMappingDAO.removeAllByGroup(configGroupId); + configGroupHostMappingDAO.removeAllByGroup(configGroupId); + configGroupDAO.removeByPK(configGroupId); + cluster.refresh(); } @Override public void addHost(Host host) throws AmbariException { - readWriteLock.writeLock().lock(); + hostLock.writeLock().lock(); try { - if (hosts != null && !hosts.isEmpty()) { - for (Host h : hosts.values()) { - if (h.getHostName().equals(host.getHostName())) { - throw new DuplicateResourceException("Host " + h.getHostName() + - "is already associated with Config Group " + - configGroupEntity.getGroupName()); - } - } - HostEntity hostEntity = hostDAO.findByName(host.getHostName()); - if (hostEntity != null) { - hosts.put(hostEntity.getHostId(), host); - } - } - } finally { - readWriteLock.writeLock().unlock(); - } - } + if (m_hosts.containsKey(host.getHostId())) { + String message = String.format( + "Host %s is already associated with the configuration group %s", host.getHostName(), + configGroupName); - @Override - public void addConfiguration(Config config) throws AmbariException { - readWriteLock.writeLock().lock(); - try { - if (configurations != null && !configurations.isEmpty()) { - for (Config c : configurations.values()) { - if (c.getType().equals(config.getType()) && c.getTag().equals - (config.getTag())) { - throw new DuplicateResourceException("Config " + config.getType() + - " with tag " + config.getTag() + " is already associated " + - "with Config Group " + configGroupEntity.getGroupName()); - } - } - configurations.put(config.getType(), config); + throw new DuplicateResourceException(message); } + + // ensure that we only update the in-memory structure if the merge was + // successful + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + persistHostMapping(Collections.singletonList(host), configGroupEntity); + m_hosts.putIfAbsent(host.getHostId(), host); } finally { - readWriteLock.writeLock().unlock(); + hostLock.writeLock().unlock(); } } @Override public ConfigGroupResponse convertToResponse() throws AmbariException { - readWriteLock.readLock().lock(); - try { - Set<Map<String, Object>> hostnames = new HashSet<Map<String, Object>>(); - for (Host host : hosts.values()) { - Map<String, Object> hostMap = new HashMap<String, Object>(); - hostMap.put("host_name", host.getHostName()); - hostnames.add(hostMap); - } + Set<Map<String, Object>> hostnames = new HashSet<Map<String, Object>>(); + for (Host host : m_hosts.values()) { + Map<String, Object> hostMap = new HashMap<String, Object>(); + hostMap.put("host_name", host.getHostName()); + hostnames.add(hostMap); + } - Set<Map<String, Object>> configObjMap = new HashSet<Map<String, Object>>(); + Set<Map<String, Object>> configObjMap = new HashSet<Map<String, Object>>(); - for (Config config : configurations.values()) { - Map<String, Object> configMap = new HashMap<String, Object>(); - configMap.put(ConfigurationResourceProvider.CONFIGURATION_CONFIG_TYPE_PROPERTY_ID, - config.getType()); - configMap.put(ConfigurationResourceProvider.CONFIGURATION_CONFIG_TAG_PROPERTY_ID, - config.getTag()); - configObjMap.add(configMap); - } - - ConfigGroupResponse configGroupResponse = new ConfigGroupResponse( - configGroupEntity.getGroupId(), cluster.getClusterName(), - configGroupEntity.getGroupName(), configGroupEntity.getTag(), - configGroupEntity.getDescription(), hostnames, configObjMap); - return configGroupResponse; - } finally { - readWriteLock.readLock().unlock(); + for (Config config : m_configurations.values()) { + Map<String, Object> configMap = new HashMap<String, Object>(); + configMap.put(ConfigurationResourceProvider.CONFIGURATION_CONFIG_TYPE_PROPERTY_ID, + config.getType()); + configMap.put(ConfigurationResourceProvider.CONFIGURATION_CONFIG_TAG_PROPERTY_ID, + config.getTag()); + configObjMap.add(configMap); } - } - @Override - @Transactional - public void refresh() { - readWriteLock.writeLock().lock(); - try { - if (isPersisted) { - ConfigGroupEntity groupEntity = configGroupDAO.findById - (configGroupEntity.getGroupId()); - configGroupDAO.refresh(groupEntity); - // TODO What other entities should refresh? - } - } finally { - readWriteLock.writeLock().unlock(); - } + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + ConfigGroupResponse configGroupResponse = new ConfigGroupResponse( + configGroupEntity.getGroupId(), cluster.getClusterName(), + configGroupEntity.getGroupName(), configGroupEntity.getTag(), + configGroupEntity.getDescription(), hostnames, configObjMap); + return configGroupResponse; } - @Override public String getServiceName() { - readWriteLock.readLock().lock(); - try { - return configGroupEntity.getServiceName(); - } finally { - readWriteLock.readLock().unlock(); - } - + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + return configGroupEntity.getServiceName(); } @Override public void setServiceName(String serviceName) { - readWriteLock.writeLock().lock(); - try { - configGroupEntity.setServiceName(serviceName); - } finally { - readWriteLock.writeLock().unlock(); - } + ConfigGroupEntity configGroupEntity = getConfigGroupEntity(); + configGroupEntity.setServiceName(serviceName); + configGroupDAO.merge(configGroupEntity); + } + /** + * Gets the {@link ConfigGroupEntity} by it's ID from the JPA cache. + * + * @return the entity. + */ + private ConfigGroupEntity getConfigGroupEntity() { + return configGroupDAO.findById(configGroupId); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/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 ed43ee1..5e887d4 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 @@ -71,7 +71,6 @@ 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; -import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.configgroup.ConfigGroup; import org.apache.ambari.server.utils.RetryHelper; import org.slf4j.Logger; @@ -558,7 +557,6 @@ public class AmbariContext { addedHost = true; if (! group.getHosts().containsKey(host.getHostId())) { group.addHost(host); - group.persistHostMapping(); } } catch (AmbariException e) { http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java index 098efa9..8a158bd 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java @@ -409,7 +409,6 @@ public class AmbariManagementControllerTest { ConfigGroup configGroup = configGroupFactory.createNew(cluster, name, tag, "", configMap, hostMap); - configGroup.persist(); cluster.addConfigGroup(configGroup); return configGroup.getId(); @@ -7011,7 +7010,6 @@ public class AmbariManagementControllerTest { ConfigGroup configGroup = cluster.getConfigGroups().get(groupId); configGroup.setHosts(new HashMap<Long, Host>() {{ put(3L, clusters.getHost(host3)); }}); - configGroup.persist(); requestId = startService(cluster1, serviceName2, false, false); mapredInstall = null; http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java index 75853db..f55bf62 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java @@ -102,7 +102,6 @@ public class ConfigGroupTest { ConfigGroup configGroup = configGroupFactory.createNew(cluster, "cg-test", "HDFS", "New HDFS configs for h1", configs, hosts); - configGroup.persist(); cluster.addConfigGroup(configGroup); return configGroup; } @@ -155,23 +154,26 @@ public class ConfigGroupTest { propertiesAttributes.put("final", attributes); Config config = configFactory.createNew(cluster, "test-site", "version100", properties, propertiesAttributes); - configGroup.addConfiguration(config); + Map<String, Config> newConfigurations = new HashMap<>(configGroup.getConfigurations()); + newConfigurations.put(config.getType(), config); + + configGroup.setConfigurations(newConfigurations); Assert.assertEquals(2, configGroup.getConfigurations().values().size()); + // re-request it and verify that the config was added + configGroupEntity = configGroupDAO.findById(configGroup.getId()); + Assert.assertEquals(2, configGroupEntity.getConfigGroupConfigMappingEntities().size()); + configGroup.setName("NewName"); configGroup.setDescription("NewDesc"); configGroup.setTag("NewTag"); // Save - configGroup.persist(); - configGroup.refresh(); configGroupEntity = configGroupDAO.findByName("NewName"); Assert.assertNotNull(configGroupEntity); - Assert.assertEquals(2, configGroupEntity - .getConfigGroupHostMappingEntities().size()); - Assert.assertEquals(2, configGroupEntity - .getConfigGroupConfigMappingEntities().size()); + Assert.assertEquals(2, configGroupEntity.getConfigGroupHostMappingEntities().size()); + Assert.assertEquals(2, configGroupEntity.getConfigGroupConfigMappingEntities().size()); Assert.assertEquals("NewTag", configGroupEntity.getTag()); Assert.assertEquals("NewDesc", configGroupEntity.getDescription()); Assert.assertNotNull(cluster.getConfig("test-site", "version100")); http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java index a3a7e11..526e462 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java @@ -252,7 +252,6 @@ public class ConfigHelperTest { LOG.info("Config group created with tag " + tag); configGroup.setTag(tag); - configGroup.persist(); cluster.addConfigGroup(configGroup); return configGroup.getId(); http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java index 69cfc9f..fc3646a 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java @@ -1332,8 +1332,6 @@ public class ClusterTest { configGroupFactory.createNew(c1, "test group", "HDFS", "descr", Collections.singletonMap("hdfs-site", config2), Collections.<Long, Host>emptyMap()); - configGroup.persist(); - c1.addConfigGroup(configGroup); scvResponse = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup); @@ -1349,7 +1347,6 @@ public class ClusterTest { configGroup.setConfigurations(Collections.singletonMap("hdfs-site", config3)); - configGroup.persist(); scvResponse = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup); assertEquals("SCV 3 should be created", Long.valueOf(3), scvResponse.getVersion()); @@ -1388,7 +1385,6 @@ public class ClusterTest { new HashMap<>(Collections.singletonMap("hdfs-site", config4)), Collections.<Long, Host>emptyMap()); - configGroup2.persist(); c1.addConfigGroup(configGroup2); scvResponse = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup2); @@ -1421,7 +1417,6 @@ public class ClusterTest { ImmutableMap.of("p1", "v2"), ImmutableMap.<String, Map<String,String>>of()); ConfigGroup configGroup = configGroupFactory.createNew(c1, "configGroup1", "version1", "test description", ImmutableMap.of(hdfsSiteConfigV2.getType(), hdfsSiteConfigV2), ImmutableMap.<Long, Host>of()); - configGroup.persist(); c1.addConfigGroup(configGroup); ServiceConfigVersionResponse hdfsSiteConfigResponseV2 = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup); @@ -1481,7 +1476,6 @@ public class ClusterTest { ImmutableMap.of("p1", "v2"), ImmutableMap.<String, Map<String,String>>of()); ConfigGroup configGroup = configGroupFactory.createNew(c1, "configGroup1", "version1", "test description", ImmutableMap.of(hdfsSiteConfigV2.getType(), hdfsSiteConfigV2), ImmutableMap.<Long, Host>of()); - configGroup.persist(); c1.addConfigGroup(configGroup); ServiceConfigVersionResponse hdfsSiteConfigResponseV2 = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup); @@ -2341,7 +2335,6 @@ public class ClusterTest { } }, Collections.<Long, Host> emptyMap()); - configGroup.persist(); cluster.addConfigGroup(configGroup); clusterEntity = clusterDAO.findByName("c1"); http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java index 8db5190..6a0457f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java @@ -561,7 +561,6 @@ public class ServiceComponentHostTest { final ConfigGroup configGroup = configGroupFactory.createNew(cluster, "cg1", "t1", "", new HashMap<String, Config>(), new HashMap<Long, Host>()); - configGroup.persist(); cluster.addConfigGroup(configGroup); Map<String, Map<String,String>> actual = @@ -822,7 +821,6 @@ public class ServiceComponentHostTest { ConfigGroup configGroup = configGroupFactory.createNew(cluster, "g1", "t1", "", new HashMap<String, Config>() {{ put("hdfs-site", c); }}, new HashMap<Long, Host>() {{ put(hostEntity.getHostId(), host); }}); - configGroup.persist(); cluster.addConfigGroup(configGroup); // HDP-x/HDFS/hdfs-site updated host to changed property @@ -875,7 +873,6 @@ public class ServiceComponentHostTest { configGroup = configGroupFactory.createNew(cluster, "g2", "t2", "", new HashMap<String, Config>() {{ put("core-site", c1); }}, new HashMap<Long, Host>() {{ put(hostEntity.getHostId(), host); }}); - configGroup.persist(); cluster.addConfigGroup(configGroup); Assert.assertTrue(sch1.convertToResponse(null).isStaleConfig()); http://git-wip-us.apache.org/repos/asf/ambari/blob/ab1c1001/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java index 68a8d4c..fac5185 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/AmbariContextTest.java @@ -445,7 +445,6 @@ public class AmbariContextTest { expect(configGroup1.getHosts()).andReturn(Collections.singletonMap(2L, host2)).once(); configGroup1.addHost(host1); - configGroup1.persistHostMapping(); // replay all mocks replayAll();