Repository: ambari Updated Branches: refs/heads/trunk ed2d58a6b -> 836ad328c
AMBARI-13894. Refactor delete configs on delete service action. (swagle) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/836ad328 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/836ad328 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/836ad328 Branch: refs/heads/trunk Commit: 836ad328cde61de996fcb10e3f67ecec2b7a36af Parents: ed2d58a Author: Siddharth Wagle <swa...@hortonworks.com> Authored: Tue Nov 17 10:22:38 2015 -0800 Committer: Siddharth Wagle <swa...@hortonworks.com> Committed: Tue Nov 17 10:22:38 2015 -0800 ---------------------------------------------------------------------- .../entities/ClusterConfigMappingEntity.java | 1 + .../orm/entities/ServiceConfigEntity.java | 7 +- .../apache/ambari/server/state/ServiceImpl.java | 27 ++-- .../server/state/cluster/ClusterImpl.java | 128 ++++++++++--------- .../server/state/cluster/ClusterTest.java | 63 ++++++++- 5 files changed, 147 insertions(+), 79 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/836ad328/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigMappingEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigMappingEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigMappingEntity.java index 5e18014..c3c3e9e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigMappingEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ClusterConfigMappingEntity.java @@ -69,6 +69,7 @@ public class ClusterConfigMappingEntity { public String getType() { return typeName; } + public void setType(String type) { typeName = type; } http://git-wip-us.apache.org/repos/asf/ambari/blob/836ad328/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceConfigEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceConfigEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceConfigEntity.java index f5bdbf9..22f82fc 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceConfigEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceConfigEntity.java @@ -89,12 +89,17 @@ public class ServiceConfigEntity { @Column(name = "host_id") private List<Long> hostIds; + /** + * Cascading remove from serviceConfig to ClusterConfig results in breaking + * the contract of configs being associated with only the cluster and the + * same config can technically belong to multiple serviceConfig versions. + */ @JoinTable( name = "serviceconfigmapping", joinColumns = {@JoinColumn(name = "service_config_id", referencedColumnName = "service_config_id")}, inverseJoinColumns = {@JoinColumn(name = "config_id", referencedColumnName = "config_id")} ) - @ManyToMany(cascade = { CascadeType.REMOVE }) + @ManyToMany private List<ClusterConfigEntity> clusterConfigEntities; @ManyToOne http://git-wip-us.apache.org/repos/asf/ambari/blob/836ad328/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java index b53e737..c13455a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java @@ -34,6 +34,7 @@ import org.apache.ambari.server.events.ServiceRemovedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.orm.dao.ClusterDAO; import org.apache.ambari.server.orm.dao.ClusterServiceDAO; +import org.apache.ambari.server.orm.dao.ConfigGroupDAO; import org.apache.ambari.server.orm.dao.ServiceConfigDAO; import org.apache.ambari.server.orm.dao.ServiceDesiredStateDAO; import org.apache.ambari.server.orm.dao.StackDAO; @@ -41,6 +42,7 @@ import org.apache.ambari.server.orm.entities.ClusterConfigEntity; import org.apache.ambari.server.orm.entities.ClusterEntity; import org.apache.ambari.server.orm.entities.ClusterServiceEntity; import org.apache.ambari.server.orm.entities.ClusterServiceEntityPK; +import org.apache.ambari.server.orm.entities.ConfigGroupEntity; import org.apache.ambari.server.orm.entities.ServiceComponentDesiredStateEntity; import org.apache.ambari.server.orm.entities.ServiceConfigEntity; import org.apache.ambari.server.orm.entities.ServiceDesiredStateEntity; @@ -49,9 +51,12 @@ import org.apache.ambari.server.orm.entities.StackEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -84,6 +89,8 @@ public class ServiceImpl implements Service { private ServiceComponentFactory serviceComponentFactory; @Inject private AmbariMetaInfo ambariMetaInfo; + @Inject + private ConfigGroupDAO configGroupDAO; /** * Data access object for retrieving stack instances. @@ -560,26 +567,14 @@ public class ServiceImpl implements Service { + ", clusterName=" + cluster.getClusterName() + ", serviceName=" + getName()); - List<ServiceConfigEntity> serviceConfigEntities = serviceConfigDAO.findByService(cluster.getClusterId(), getName()); + List<ServiceConfigEntity> serviceConfigEntities = + serviceConfigDAO.findByService(cluster.getClusterId(), getName()); - Long maxServiceConfigEntityId = -1L; - ServiceConfigEntity lastServiceConfigEntity = null; // last service config by id, should have all needed clusterConfigEntities - for (ServiceConfigEntity serviceConfigEntity : serviceConfigEntities) { - if(serviceConfigEntity.getServiceConfigId() > maxServiceConfigEntityId) { - maxServiceConfigEntityId = serviceConfigEntity.getServiceConfigId(); - lastServiceConfigEntity = serviceConfigEntity; - } + // Only delete the historical version information and not original + // config data serviceConfigDAO.remove(serviceConfigEntity); } - - if(lastServiceConfigEntity != null) { - List<String> configTypesToDisable = new ArrayList<String>(); - for(ClusterConfigEntity clusterConfigEntity:lastServiceConfigEntity.getClusterConfigEntities()) { - configTypesToDisable.add(clusterConfigEntity.getType()); - } - clusterDAO.removeClusterConfigMappingEntityByTypes(cluster.getClusterId(), configTypesToDisable); - } } @Override http://git-wip-us.apache.org/repos/asf/ambari/blob/836ad328/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 e7c1b19..0be04f3 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 @@ -788,9 +788,9 @@ public class ClusterImpl implements Cluster { if (schToRemove == null) { LOG.warn("Unavailable in per host cache. ServiceComponentHost" - + ", serviceName=" + serviceName - + ", serviceComponentName" + componentName - + ", hostname= " + hostname); + + ", serviceName=" + serviceName + + ", serviceComponentName" + componentName + + ", hostname= " + hostname); } if (LOG.isDebugEnabled()) { @@ -1182,7 +1182,7 @@ public class ClusterImpl implements Cluster { // find any hosts that do not have the stack/repo version already Sets.SetView<String> hostsMissingRepoVersion = Sets.difference( - hosts.keySet(), existingHostsWithClusterStackAndVersion); + hosts.keySet(), existingHostsWithClusterStackAndVersion); for (String hostname : hosts.keySet()) { // if the host is in maintenance mode, that's an explicit marker which @@ -1264,7 +1264,7 @@ public class ClusterImpl implements Cluster { // Also returns when have a mix of CURRENT and INSTALLING|INSTALLED|UPGRADING|UPGRADED LOG.warn("have a mix of CURRENT and INSTALLING|INSTALLED|UPGRADING|UPGRADED host versions, " + - "returning OUT_OF_SYNC as cluster version. Host version states: " + stateToHosts.toString()); + "returning OUT_OF_SYNC as cluster version. Host version states: " + stateToHosts.toString()); return RepositoryVersionState.OUT_OF_SYNC; } @@ -1430,8 +1430,8 @@ public class ClusterImpl implements Cluster { StackId repoVersionStackId = new StackId(repoVersionStackEntity); HostVersionEntity hostVersionEntity = hostVersionDAO.findByClusterStackVersionAndHost( - getClusterName(), repoVersionStackId, repositoryVersion.getVersion(), - host.getHostName()); + getClusterName(), repoVersionStackId, repositoryVersion.getVersion(), + host.getHostName()); hostTransitionStateWriteLock.lock(); try { @@ -2171,9 +2171,9 @@ public class ClusterImpl implements Cluster { } configChangeLog.info("Cluster '{}' changed by: '{}'; service_name='{}' config_group='{}' config_group_id='{}' " + - "version='{}'", getClusterName(), user, serviceName, - configGroup==null?"default":configGroup.getName(), - configGroup==null?"-1":configGroup.getId(), + "version='{}'", getClusterName(), user, serviceName, + configGroup == null ? "default" : configGroup.getName(), + configGroup == null ? "-1" : configGroup.getId(), serviceConfigEntity.getVersion()); String configGroupName = configGroup != null ? configGroup.getName() : null; @@ -2994,69 +2994,77 @@ public class ClusterImpl implements Cluster { return new HashMap<>(); } - /** - * {@inheritDoc} - */ - @Override + // The caller should make sure global write lock is acquired. @Transactional - public void removeConfigurations(StackId stackId) { - clusterGlobalLock.writeLock().lock(); - try { - long clusterId = clusterEntity.getClusterId(); + void removeAllConfigsForStack(StackId stackId) { + long clusterId = clusterEntity.getClusterId(); - // this will keep track of cluster config mappings that need removal - // since there is no relationship between configs and their mappings, we - // have to do it manually - List<ClusterConfigEntity> removedClusterConfigs = new ArrayList<ClusterConfigEntity>(50); - Collection<ClusterConfigEntity> clusterConfigEntities = clusterEntity.getClusterConfigEntities(); + // this will keep track of cluster config mappings that need removal + // since there is no relationship between configs and their mappings, we + // have to do it manually + List<ClusterConfigEntity> removedClusterConfigs = new ArrayList<ClusterConfigEntity>(50); + Collection<ClusterConfigEntity> clusterConfigEntities = clusterEntity.getClusterConfigEntities(); - List<ClusterConfigEntity> clusterConfigs = clusterDAO.getAllConfigurations( - clusterId, stackId); + List<ServiceConfigEntity> serviceConfigs = serviceConfigDAO.getAllServiceConfigsForClusterAndStack( + clusterId, stackId); - // remove any lefover cluster configurations that don't have a service - // configuration (like cluster-env) - for (ClusterConfigEntity clusterConfig : clusterConfigs) { - clusterDAO.removeConfig(clusterConfig); - clusterConfigEntities.remove(clusterConfig); + // remove all service configurations and associated configs + Collection<ServiceConfigEntity> serviceConfigEntities = clusterEntity.getServiceConfigEntities(); - removedClusterConfigs.add(clusterConfig); + for (ServiceConfigEntity serviceConfig : serviceConfigs) { + for (ClusterConfigEntity configEntity : serviceConfig.getClusterConfigEntities()) { + clusterConfigEntities.remove(configEntity); + clusterDAO.removeConfig(configEntity); + removedClusterConfigs.add(configEntity); } + serviceConfigDAO.remove(serviceConfig); + serviceConfigEntities.remove(serviceConfig); + } - clusterEntity = clusterDAO.merge(clusterEntity); - - List<ServiceConfigEntity> serviceConfigs = serviceConfigDAO.getAllServiceConfigsForClusterAndStack( - clusterId, stackId); + // remove any lefover cluster configurations that don't have a service + // configuration (like cluster-env) + List<ClusterConfigEntity> clusterConfigs = clusterDAO.getAllConfigurations( + clusterId, stackId); - // remove all service configurations - Collection<ServiceConfigEntity> serviceConfigEntities = clusterEntity.getServiceConfigEntities(); - for (ServiceConfigEntity serviceConfig : serviceConfigs) { - serviceConfigDAO.remove(serviceConfig); - serviceConfigEntities.remove(serviceConfig); - } + for (ClusterConfigEntity clusterConfig : clusterConfigs) { + clusterConfigEntities.remove(clusterConfig); + clusterDAO.removeConfig(clusterConfig); + removedClusterConfigs.add(clusterConfig); + } - clusterEntity = clusterDAO.merge(clusterEntity); + clusterEntity = clusterDAO.merge(clusterEntity); - // remove config mappings - Collection<ClusterConfigMappingEntity> configMappingEntities = clusterEntity.getConfigMappingEntities(); - for (ClusterConfigEntity removedClusterConfig : removedClusterConfigs) { - String removedClusterConfigType = removedClusterConfig.getType(); - String removedClusterConfigTag = removedClusterConfig.getTag(); - - Iterator<ClusterConfigMappingEntity> clusterConfigMappingIterator = configMappingEntities.iterator(); - while (clusterConfigMappingIterator.hasNext()) { - ClusterConfigMappingEntity clusterConfigMapping = clusterConfigMappingIterator.next(); - String mappingType = clusterConfigMapping.getType(); - String mappingTag = clusterConfigMapping.getTag(); - - if (removedClusterConfigTag.equals(mappingTag) - && removedClusterConfigType.equals(mappingType)) { - clusterConfigMappingIterator.remove(); - clusterDAO.removeConfigMapping(clusterConfigMapping); - } + // remove config mappings + Collection<ClusterConfigMappingEntity> configMappingEntities = clusterEntity.getConfigMappingEntities(); + for (ClusterConfigEntity removedClusterConfig : removedClusterConfigs) { + String removedClusterConfigType = removedClusterConfig.getType(); + String removedClusterConfigTag = removedClusterConfig.getTag(); + + Iterator<ClusterConfigMappingEntity> clusterConfigMappingIterator = configMappingEntities.iterator(); + while (clusterConfigMappingIterator.hasNext()) { + ClusterConfigMappingEntity clusterConfigMapping = clusterConfigMappingIterator.next(); + String mappingType = clusterConfigMapping.getType(); + String mappingTag = clusterConfigMapping.getTag(); + + if (removedClusterConfigTag.equals(mappingTag) + && removedClusterConfigType.equals(mappingType)) { + clusterConfigMappingIterator.remove(); + clusterDAO.removeConfigMapping(clusterConfigMapping); } } + } - clusterEntity = clusterDAO.merge(clusterEntity); + clusterEntity = clusterDAO.merge(clusterEntity); + } + + /** + * {@inheritDoc} + */ + @Override + public void removeConfigurations(StackId stackId) { + clusterGlobalLock.writeLock().lock(); + try { + removeAllConfigsForStack(stackId); cacheConfigurations(); } finally { http://git-wip-us.apache.org/repos/asf/ambari/blob/836ad328/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 edd6267..28ba0e7 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 @@ -896,12 +896,12 @@ public class ClusterTest { Assert.assertTrue("Expect desired config contain " + config1.getType(), desiredConfigs.containsKey("global")); Assert.assertTrue("Expect desired config contain " + config3.getType(), desiredConfigs.containsKey("core-site")); Assert.assertEquals("Expect desired config for global should be " + config1.getTag(), - config1.getTag(), desiredConfigs.get(config1.getType()).getTag()); + config1.getTag(), desiredConfigs.get(config1.getType()).getTag()); Assert.assertEquals("_test1", desiredConfigs.get(config1.getType()).getUser()); Assert.assertEquals("_test3", desiredConfigs.get(config3.getType()).getUser()); DesiredConfig dc = desiredConfigs.get(config1.getType()); Assert.assertTrue("Expect no host-level overrides", - (null == dc.getHostOverrides() || dc.getHostOverrides().size() == 0)); + (null == dc.getHostOverrides() || dc.getHostOverrides().size() == 0)); c1.addDesiredConfig("_test2", Collections.singleton(config2)); Assert.assertEquals("_test2", c1.getDesiredConfigs().get(config2.getType()).getUser()); @@ -1011,6 +1011,65 @@ public class ClusterTest { } @Test + public void testDeleteServiceWithConfigHistory() throws Exception { + createDefaultCluster(); + + c1.addService("HDFS").persist(); + + Config config1 = configFactory.createNew(c1, "hdfs-site", + new HashMap<String, String>() {{ put("a", "b"); }}, new HashMap<String, Map<String,String>>()); + config1.setTag("version1"); + + Config config2 = configFactory.createNew(c1, "core-site", + new HashMap<String, String>() {{ put("x", "y"); }}, new HashMap<String, Map<String,String>>()); + config2.setTag("version2"); + + config1.persist(); + c1.addConfig(config1); + config2.persist(); + c1.addConfig(config2); + + Set<Config> configs = new HashSet<Config>(); + configs.add(config1); + configs.add(config2); + + c1.addDesiredConfig("admin", configs); + List<ServiceConfigVersionResponse> serviceConfigVersions = c1.getServiceConfigVersions(); + Assert.assertNotNull(serviceConfigVersions); + // Single serviceConfigVersion for multiple configs + Assert.assertEquals(1, serviceConfigVersions.size()); + Assert.assertEquals(Long.valueOf(1), serviceConfigVersions.get(0).getVersion()); + Assert.assertEquals(2, c1.getDesiredConfigs().size()); + Assert.assertEquals("version1", c1.getDesiredConfigByType("hdfs-site").getTag()); + Assert.assertEquals("version2", c1.getDesiredConfigByType("core-site").getTag()); + + Map<String, Collection<ServiceConfigVersionResponse>> activeServiceConfigVersions = + c1.getActiveServiceConfigVersions(); + Assert.assertEquals(1, activeServiceConfigVersions.size()); + + c1.deleteService("HDFS"); + + Assert.assertEquals(0, c1.getServices().size()); + Assert.assertEquals(0, c1.getServiceConfigVersions().size()); + + EntityManager em = injector.getProvider(EntityManager.class).get(); + + // ServiceConfig + Assert.assertEquals(0, + em.createQuery("SELECT serviceConfig from ServiceConfigEntity serviceConfig").getResultList().size()); + // ClusterConfig + Assert.assertEquals(2, + em.createQuery("SELECT config from ClusterConfigEntity config").getResultList().size()); + // ClusterConfigMapping + Assert.assertEquals(2, + em.createQuery("SELECT configmapping from ClusterConfigMappingEntity configmapping").getResultList().size()); + + // ServiceConfigMapping + Assert.assertEquals(0, + em.createNativeQuery("SELECT * from serviceconfigmapping").getResultList().size()); + } + + @Test public void testGetHostsDesiredConfigs() throws Exception { createDefaultCluster();
