Repository: ambari Updated Branches: refs/heads/branch-2.4 fa7c8effb -> d3cf46b0d
AMBARI-18580. Delete host throws 500 Error with unique constraint violation. (mpapirkovskyy) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/d3cf46b0 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/d3cf46b0 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/d3cf46b0 Branch: refs/heads/branch-2.4 Commit: d3cf46b0d47979e949a4418187a7af81f1bd0cb8 Parents: fa7c8ef Author: Myroslav Papirkovskyi <[email protected]> Authored: Wed Nov 2 20:15:43 2016 +0200 Committer: Myroslav Papirkovskyi <[email protected]> Committed: Thu Nov 3 19:28:09 2016 +0200 ---------------------------------------------------------------------- .../internal/HostResourceProvider.java | 1 - .../orm/dao/ConfigGroupHostMappingDAO.java | 7 +- .../apache/ambari/server/state/Clusters.java | 7 + .../server/state/cluster/ClustersImpl.java | 170 ++++++++++--------- .../state/configgroup/ConfigGroupImpl.java | 14 +- .../ambari/server/topology/LogicalRequest.java | 4 +- .../AmbariManagementControllerTest.java | 1 + .../server/controller/KerberosHelperTest.java | 3 + .../ambari/server/state/ConfigHelperTest.java | 2 + .../ambari/server/utils/StageUtilsTest.java | 5 + 10 files changed, 127 insertions(+), 87 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java index 6d555e6..9dea83c 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java @@ -832,7 +832,6 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { } } - @Transactional protected DeleteStatusMetaData deleteHosts(Set<HostRequest> requests, boolean dryRun) throws AmbariException { http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ConfigGroupHostMappingDAO.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ConfigGroupHostMappingDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ConfigGroupHostMappingDAO.java index 71d93cc..baeae9d 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ConfigGroupHostMappingDAO.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ConfigGroupHostMappingDAO.java @@ -289,6 +289,7 @@ public class ConfigGroupHostMappingDAO { @Transactional public void removeAllByHost(Long hostId) { + populateCache(); TypedQuery<String> query = entityManagerProvider.get().createQuery ("DELETE FROM ConfigGroupHostMappingEntity confighosts WHERE " + "confighosts.hostId = ?1", String.class); @@ -297,8 +298,10 @@ public class ConfigGroupHostMappingDAO { Set<ConfigGroupHostMapping> setByHost = configGroupHostMappingByHost.get(hostId); - - setByHost.clear(); + + if (setByHost != null) { + setByHost.clear(); + } } private ConfigGroupHostMapping buildConfigGroupHostMapping( http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java index 2d859b3..4939e0e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java @@ -18,6 +18,7 @@ package org.apache.ambari.server.state; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; @@ -243,6 +244,12 @@ public interface Clusters { throws AmbariException; /** + * Deletes hosts in bulk + * @param hostnames list of hostnames to delete + */ + void deleteHosts(Collection<String> hostnames) throws AmbariException; + + /** * Determine whether or not access to the cluster resource identified * by the given cluster name should be allowed based on the permissions * granted to the current user. http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java index 6318545..8285065 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java @@ -45,6 +45,7 @@ import org.apache.ambari.server.events.HostRemovedEvent; import org.apache.ambari.server.events.publishers.AmbariEventPublisher; import org.apache.ambari.server.orm.dao.ClusterDAO; import org.apache.ambari.server.orm.dao.ClusterVersionDAO; +import org.apache.ambari.server.orm.dao.ConfigGroupHostMappingDAO; import org.apache.ambari.server.orm.dao.HostConfigMappingDAO; import org.apache.ambari.server.orm.dao.HostDAO; import org.apache.ambari.server.orm.dao.HostRoleCommandDAO; @@ -135,6 +136,8 @@ public class ClustersImpl implements Clusters { @Inject private KerberosPrincipalHostDAO kerberosPrincipalHostDAO; @Inject + private ConfigGroupHostMappingDAO configGroupHostMappingDAO; + @Inject private HostConfigMappingDAO hostConfigMappingDAO; @Inject private ServiceConfigDAO serviceConfigDAO; @@ -861,114 +864,125 @@ public class ClustersImpl implements Clusters { */ @Override public void deleteHost(String hostname) throws AmbariException { - // unmapping hosts from a cluster modifies the collections directly; keep - // a copy of this to ensure that we can pass in the original set of - // clusters that the host belonged to to the host removal event - Set<Cluster> clusters = hostClusterMap.get(hostname); - if (clusters == null) { - throw new HostNotFoundException(hostname); - } + deleteHosts(Collections.singletonList(hostname)); + } - Set<Cluster> hostsClusters = new HashSet<>(clusters); + @Override + public void deleteHosts(Collection<String> hostnames) throws AmbariException { + checkLoaded(); + w.lock(); + try { + for (String hostname : hostnames) { + if (!hosts.containsKey(hostname)) { + throw new HostNotFoundException("Could not find host " + hostname); + } + } - deleteHostEntityRelationships(hostname); + deleteHostEntities(hostnames); - // Publish the event, using the original list of clusters that the host - // belonged to - HostRemovedEvent event = new HostRemovedEvent(hostname, hostsClusters); - eventPublisher.publish(event); - } + for (String hostname : hostnames) { + //cleanup in-memory maps after transaction commit + Set<Cluster> clusters = new HashSet<>(hostClusterMap.get(hostname)); - /*** - * Deletes all of the JPA relationships between a host and other entities. - * This method will not fire {@link HostRemovedEvent} since it is performed - * within an {@link Transactional} and the event must fire after the - * transaction is successfully committed. - * - * @param hostname - * @throws AmbariException - */ - @Transactional - void deleteHostEntityRelationships(String hostname) throws AmbariException { - checkLoaded(); - if (!hosts.containsKey(hostname)) { - throw new HostNotFoundException("Could not find host " + hostname); - } + Host host = hosts.get(hostname); + hosts.remove(hostname); + hostsById.remove(host.getHostId()); //hostentity is not retrieved from session implicitly so we can use this - w.lock(); + for (Cluster cluster : clusters) { + hostClusterMap.get(hostname).remove(cluster); + clusterHostMap.get(cluster.getClusterName()).remove(host); + } - try { - HostEntity entity = hostDAO.findByName(hostname); + deleteConfigGroupHostMapping(host.getHostId()); - if (entity == null) { - return; - } - // Remove from all clusters in the cluster_host_mapping table. - // This will also remove from kerberos_principal_hosts, hostconfigmapping, and configgrouphostmapping - Set<Cluster> clusters = hostClusterMap.get(hostname); - Set<Long> clusterIds = Sets.newHashSet(); - for (Cluster cluster: clusters) { - clusterIds.add(cluster.getClusterId()); + // Publish the event, using the original list of clusters that the host + // belonged to + HostRemovedEvent event = new HostRemovedEvent(hostname, clusters); + eventPublisher.publish(event); } + } finally { + w.unlock(); + } + } + @Transactional + //do not do in-memory modifications in this method, to safely rollback in case of failure + void deleteHostEntities(Collection<String> hostnames) throws AmbariException { + checkLoaded(); + for (String hostname : hostnames) { + try { + HostEntity entity = hostDAO.findByName(hostname); + if (entity == null) { + return; + } + // Remove from all clusters in the cluster_host_mapping table. + // This will also remove from kerberos_principal_hosts, hostconfigmapping, and configgrouphostmapping + Set<Cluster> clusters = hostClusterMap.get(hostname); + Set<Long> clusterIds = Sets.newHashSet(); + for (Cluster cluster: clusters) { + clusterIds.add(cluster.getClusterId()); + } - unmapHostFromClusters(hostname, clusters); - hostDAO.refresh(entity); + for (Long clusterId : clusterIds) { + unmapHostClusterEntities(hostname, clusterId); + } - hostVersionDAO.removeByHostName(hostname); + hostVersionDAO.removeByHostName(hostname); - // Remove blueprint tasks before hostRoleCommands - // TopologyLogicalTask owns the OneToOne relationship but Cascade is on HostRoleCommandEntity - if (entity.getHostRoleCommandEntities() != null) { - for (HostRoleCommandEntity hrcEntity : entity.getHostRoleCommandEntities()) { - TopologyLogicalTaskEntity topologyLogicalTaskEnity = hrcEntity.getTopologyLogicalTaskEntity(); - if (topologyLogicalTaskEnity != null) { - topologyLogicalTaskDAO.remove(topologyLogicalTaskEnity); - hrcEntity.setTopologyLogicalTaskEntity(null); + // Remove blueprint tasks before hostRoleCommands + // TopologyLogicalTask owns the OneToOne relationship but Cascade is on HostRoleCommandEntity + if (entity.getHostRoleCommandEntities() != null) { + for (HostRoleCommandEntity hrcEntity : entity.getHostRoleCommandEntities()) { + TopologyLogicalTaskEntity topologyLogicalTaskEnity = hrcEntity.getTopologyLogicalTaskEntity(); + if (topologyLogicalTaskEnity != null) { + topologyLogicalTaskDAO.remove(topologyLogicalTaskEnity); + hrcEntity.setTopologyLogicalTaskEntity(null); + } } } - } - for (Long clusterId: clusterIds) { - for (TopologyRequestEntity topologyRequestEntity: topologyRequestDAO.findByClusterId(clusterId)) { - TopologyLogicalRequestEntity topologyLogicalRequestEntity = topologyRequestEntity.getTopologyLogicalRequestEntity(); - for (TopologyHostRequestEntity topologyHostRequestEntity: topologyLogicalRequestEntity.getTopologyHostRequestEntities()) { - if (hostname.equals(topologyHostRequestEntity.getHostName())) { - topologyHostRequestDAO.remove(topologyHostRequestEntity); + for (Long clusterId: clusterIds) { + for (TopologyRequestEntity topologyRequestEntity: topologyRequestDAO.findByClusterId(clusterId)) { + TopologyLogicalRequestEntity topologyLogicalRequestEntity = topologyRequestEntity.getTopologyLogicalRequestEntity(); + + for (TopologyHostRequestEntity topologyHostRequestEntity: topologyLogicalRequestEntity.getTopologyHostRequestEntities()) { + if (hostname.equals(topologyHostRequestEntity.getHostName())) { + topologyHostRequestDAO.remove(topologyHostRequestEntity); + } } } } - } + entity.setHostRoleCommandEntities(null); + hostRoleCommandDAO.removeByHostId(entity.getHostId()); - entity.setHostRoleCommandEntities(null); - hostRoleCommandDAO.removeByHostId(entity.getHostId()); + entity.setHostStateEntity(null); + hostStateDAO.removeByHostId(entity.getHostId()); + hostConfigMappingDAO.removeByHostId(entity.getHostId()); + serviceConfigDAO.removeHostFromServiceConfigs(entity.getHostId()); + requestOperationLevelDAO.removeByHostId(entity.getHostId()); + topologyHostInfoDAO.removeByHost(entity); - entity.setHostStateEntity(null); - hostStateDAO.removeByHostId(entity.getHostId()); - hostConfigMappingDAO.removeByHostId(entity.getHostId()); - serviceConfigDAO.removeHostFromServiceConfigs(entity.getHostId()); - requestOperationLevelDAO.removeByHostId(entity.getHostId()); - topologyHostInfoDAO.removeByHost(entity); + kerberosPrincipalHostDAO.removeByHost(entity.getHostId()); - // Remove from dictionaries - hosts.remove(hostname); - hostsById.remove(entity.getHostId()); + configGroupHostMappingDAO.removeAllByHost(entity.getHostId()); - hostDAO.remove(entity); + hostDAO.remove(entity); + + // Note, if the host is still heartbeating, then new records will be re-inserted + // into the hosts and hoststate tables + } catch (Exception e) { + throw new AmbariException("Could not remove host", e); + } - // Note, if the host is still heartbeating, then new records will be re-inserted - // into the hosts and hoststate tables - } catch (Exception e) { - throw new AmbariException("Could not remove host", e); - } finally { - w.unlock(); } + } + @Override public boolean checkPermission(String clusterName, boolean readOnly) { http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/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 1d6b1e8..ccca9fc 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 @@ -295,11 +295,17 @@ public class ConfigGroupImpl implements ConfigGroup { 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 = new ConfigGroupHostMappingEntityPK(); - hostMappingEntityPK.setHostId(hostId); - hostMappingEntityPK.setConfigGroupId(configGroupEntity.getGroupId()); + hostMappingEntityPK.setHostId(hostId); + hostMappingEntityPK.setConfigGroupId(configGroupEntity.getGroupId()); + + ConfigGroupHostMappingEntity mappingEntity = configGroupHostMappingDAO.findByPK(hostMappingEntityPK); + if (mappingEntity == null) { + // tolerate missing mapping as it could be removed with HostEntity + return; + } + try { configGroupHostMappingDAO.removeByPK(hostMappingEntityPK); } catch (Exception e) { LOG.error("Failed to delete config group host mapping" http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java index 3aaf589..de7b83a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java @@ -344,7 +344,7 @@ public class LogicalRequest extends Request { Iterator<HostRequest> hostRequestIterator = outstandingHostRequests.iterator(); while (hostRequestIterator.hasNext()) { - if (hostRequestIterator.next().getHostName().equals(hostName)) { + if (StringUtils.equals(hostRequestIterator.next().getHostName(), hostName)) { hostRequestIterator.remove(); break; } @@ -353,7 +353,7 @@ public class LogicalRequest extends Request { //todo: synchronization Iterator<HostRequest> allHostRequesIterator = allHostRequests.iterator(); while (allHostRequesIterator.hasNext()) { - if (allHostRequesIterator.next().getHostName().equals(hostName)) { + if (StringUtils.equals(allHostRequesIterator.next().getHostName(), hostName)) { allHostRequesIterator.remove(); break; } http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/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 3ad1f1f..1ea9407 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 @@ -9025,6 +9025,7 @@ public class AmbariManagementControllerTest { try { HostResourceProviderTest.deleteHosts(controller, requests); } catch (Exception e) { + LOG.error("Fail to remove host", e); fail("Did not expect an error deleting the host from the cluster. Error: " + e.getMessage()); } // Verify host is no longer part of the cluster http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java index 3c97ce9..dde8ce2 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java @@ -74,6 +74,7 @@ import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.State; import org.apache.ambari.server.state.cluster.ClusterFactory; import org.apache.ambari.server.state.cluster.ClustersImpl; +import org.apache.ambari.server.state.configgroup.ConfigGroupFactory; import org.apache.ambari.server.state.host.HostFactory; import org.apache.ambari.server.state.kerberos.KerberosComponentDescriptor; import org.apache.ambari.server.state.kerberos.KerberosConfigurationDescriptor; @@ -219,6 +220,8 @@ public class KerberosHelperTest extends EasyMockSupport { bind(HostRoleCommandDAO.class).toInstance(createNiceMock(HostRoleCommandDAO.class)); bind(AuditLogger.class).toInstance(createNiceMock(AuditLogger.class)); bind(ArtifactDAO.class).toInstance(createNiceMock(ArtifactDAO.class)); + + bind(ConfigGroupFactory.class).toInstance(createNiceMock(ConfigGroupFactory.class)); } }); http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/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 37a48f0..ce0ca71 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 @@ -845,6 +845,8 @@ public class ConfigHelperTest { bind(ClusterController.class).toInstance(clusterController); bind(StackManagerFactory.class).toInstance(createNiceMock(StackManagerFactory.class)); bind(HostRoleCommandDAO.class).toInstance(createNiceMock(HostRoleCommandDAO.class)); + + bind(ConfigGroupFactory.class).toInstance(createNiceMock(ConfigGroupFactory.class)); } }); http://git-wip-us.apache.org/repos/asf/ambari/blob/d3cf46b0/ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java b/ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java index 5d39841..4382ba8 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/utils/StageUtilsTest.java @@ -74,6 +74,9 @@ import org.apache.ambari.server.state.ServiceComponentHostFactory; import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.cluster.ClusterFactory; import org.apache.ambari.server.state.cluster.ClustersImpl; +import org.apache.ambari.server.state.configgroup.ConfigGroup; +import org.apache.ambari.server.state.configgroup.ConfigGroupFactory; +import org.apache.ambari.server.state.configgroup.ConfigGroupImpl; import org.apache.ambari.server.state.host.HostFactory; import org.apache.ambari.server.state.stack.OsFamily; import org.apache.ambari.server.topology.PersistedState; @@ -108,6 +111,8 @@ public class StageUtilsTest extends EasyMockSupport { @Override protected void configure() { + bind(ConfigGroupFactory.class).toInstance(createNiceMock(ConfigGroupFactory.class)); + bind(EntityManager.class).toInstance(createNiceMock(EntityManager.class)); bind(DBAccessor.class).toInstance(createNiceMock(DBAccessor.class)); bind(ClusterFactory.class).toInstance(createNiceMock(ClusterFactory.class));
