You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by sm...@apache.org on 2015/12/07 01:28:25 UTC
ambari git commit: AMBARI-14188. During Upgrade Topology Manager
Causes Ambari To Be Unresponsive With Infinite Loop (Sebastian Toader via
smohanty)
Repository: ambari
Updated Branches:
refs/heads/trunk 4d58b1a71 -> 4f0acbf1a
AMBARI-14188. During Upgrade Topology Manager Causes Ambari To Be Unresponsive With Infinite Loop (Sebastian Toader via smohanty)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/4f0acbf1
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/4f0acbf1
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/4f0acbf1
Branch: refs/heads/trunk
Commit: 4f0acbf1a786e07a5f76009b37b3e51cd0a6283c
Parents: 4d58b1a
Author: Sumit Mohanty <sm...@hortonworks.com>
Authored: Sun Dec 6 16:06:35 2015 -0800
Committer: Sumit Mohanty <sm...@hortonworks.com>
Committed: Sun Dec 6 16:06:35 2015 -0800
----------------------------------------------------------------------
.../org/apache/ambari/server/state/Cluster.java | 8 +-
.../ambari/server/state/DesiredConfig.java | 56 +++++++++-
.../server/state/cluster/ClusterImpl.java | 57 ++++++++--
.../ambari/server/topology/AmbariContext.java | 49 ++++++++-
.../ambari/server/topology/TopologyManager.java | 4 +-
.../ambari/server/state/DesiredConfigTest.java | 17 +++
.../server/topology/AmbariContextTest.java | 107 +++++++++++++++++++
7 files changed, 282 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java
index 069eb1b..2c5c5af 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java
@@ -404,11 +404,17 @@ public interface Cluster {
*/
boolean isConfigTypeExists(String configType);
/**
- * Gets the desired configurations for the cluster.
+ * Gets the active desired configurations for the cluster.
* @return a map of type-to-configuration information.
*/
Map<String, DesiredConfig> getDesiredConfigs();
+ /**
+ * Gets all versions of the desired configurations for the cluster.
+ * @return a map of type-to-configuration information.
+ */
+ Map<String, Set<DesiredConfig>> getAllDesiredConfigVersions();
+
/**
* Creates a cluster response based on the current cluster definition
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/ambari-server/src/main/java/org/apache/ambari/server/state/DesiredConfig.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/DesiredConfig.java b/ambari-server/src/main/java/org/apache/ambari/server/state/DesiredConfig.java
index 0635284..80d05f7 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/DesiredConfig.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/DesiredConfig.java
@@ -20,6 +20,8 @@ package org.apache.ambari.server.state;
import java.util.ArrayList;
import java.util.List;
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
import org.codehaus.jackson.annotate.JsonProperty;
import org.codehaus.jackson.map.annotate.JsonSerialize;
import org.codehaus.jackson.map.annotate.JsonSerialize.Inclusion;
@@ -119,9 +121,9 @@ public class DesiredConfig {
* Used to represent an override on a host.
*/
//TODO include changes for config versions
- public static class HostOverride {
- private String hostName;
- private String versionOverrideTag;
+ public final static class HostOverride {
+ private final String hostName;
+ private final String versionOverrideTag;
/**
* @param name the host name
@@ -148,6 +150,27 @@ public class DesiredConfig {
return versionOverrideTag;
}
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+
+ if (o == null || getClass() != o.getClass()) return false;
+
+ HostOverride that = (HostOverride) o;
+
+ return new EqualsBuilder()
+ .append(hostName, that.hostName)
+ .append(versionOverrideTag, that.versionOverrideTag)
+ .isEquals();
+ }
+
+ @Override
+ public int hashCode() {
+ return new HashCodeBuilder(17, 37)
+ .append(hostName)
+ .append(versionOverrideTag)
+ .toHashCode();
+ }
}
@Override
@@ -174,4 +197,31 @@ public class DesiredConfig {
return sb.toString();
}
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+
+ if (o == null || getClass() != o.getClass()) return false;
+
+ DesiredConfig that = (DesiredConfig) o;
+
+ return new EqualsBuilder()
+ .append(tag, that.tag)
+ .append(serviceName, that.serviceName)
+ .append(user, that.user)
+ .append(version, that.version)
+ .append(hostOverrides, that.hostOverrides)
+ .isEquals();
+ }
+
+ @Override
+ public int hashCode() {
+ return new HashCodeBuilder(17, 37)
+ .append(tag)
+ .append(serviceName)
+ .append(user)
+ .append(version)
+ .append(hostOverrides)
+ .toHashCode();
+ }
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/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 f8fa8fa..1078343 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
@@ -36,8 +36,10 @@ import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
+import javax.annotation.Nullable;
import javax.persistence.RollbackException;
+import com.google.common.collect.Maps;
import org.apache.ambari.server.AmbariException;
import org.apache.ambari.server.ConfigGroupNotFoundException;
import org.apache.ambari.server.DuplicateResourceException;
@@ -2092,16 +2094,50 @@ public class ClusterImpl implements Cluster {
}
}
+ /**
+ * Gets all versions of the desired configurations for the cluster.
+ * @return a map of type-to-configuration information.
+ */
+ @Override
+ public Map<String, Set<DesiredConfig>> getAllDesiredConfigVersions() {
+ return getDesiredConfigs(true);
+ }
+
+ /**
+ * Gets the active desired configurations for the cluster.
+ * @return a map of type-to-configuration information.
+ */
@Override
public Map<String, DesiredConfig> getDesiredConfigs() {
+ Map<String, Set<DesiredConfig>> activeConfigsByType = getDesiredConfigs(false);
+
+ return Maps.transformEntries(
+ activeConfigsByType,
+ new Maps.EntryTransformer<String, Set<DesiredConfig>, DesiredConfig>() {
+ @Override
+ public DesiredConfig transformEntry(@Nullable String key, @Nullable Set<DesiredConfig> value) {
+ return value.iterator().next();
+ }
+ });
+ }
+
+
+ /**
+ * Gets desired configurations for the cluster.
+ * @param allVersions specifies if all versions of the desired configurations to be returned
+ * or only the active ones. It is expected that there is one and only one active
+ * desired configuration per config type.
+ * @return a map of type-to-configuration information.
+ */
+ private Map<String, Set<DesiredConfig>> getDesiredConfigs(boolean allVersions) {
loadConfigurations();
clusterGlobalLock.readLock().lock();
try {
- Map<String, DesiredConfig> map = new HashMap<String, DesiredConfig>();
- Collection<String> types = new HashSet<String>();
+ Map<String, Set<DesiredConfig>> map = new HashMap<>();
+ Collection<String> types = new HashSet<>();
for (ClusterConfigMappingEntity e : clusterDAO.getClusterConfigMappingEntitiesByCluster(getClusterId())) {
- if (e.isSelected() > 0) {
+ if (allVersions || e.isSelected() > 0) {
DesiredConfig c = new DesiredConfig();
c.setServiceName(null);
c.setTag(e.getTag());
@@ -2113,7 +2149,13 @@ public class ClusterImpl implements Cluster {
}
c.setVersion(allConfigs.get(e.getType()).get(e.getTag()).getVersion());
- map.put(e.getType(), c);
+ Set<DesiredConfig> configs = map.get(e.getType());
+ if (configs == null)
+ configs = new HashSet<>();
+
+ configs.add(c);
+
+ map.put(e.getType(), configs);
types.add(e.getType());
}
}
@@ -2125,7 +2167,7 @@ public class ClusterImpl implements Cluster {
Map<String, List<HostConfigMapping>> hostMappingsByType = hostConfigMappingDAO.findSelectedHostsByTypes(
clusterEntity.getClusterId(), types);
- for (Entry<String, DesiredConfig> entry : map.entrySet()) {
+ for (Entry<String, Set<DesiredConfig>> entry : map.entrySet()) {
List<DesiredConfig.HostOverride> hostOverrides = new ArrayList<DesiredConfig.HostOverride>();
for (HostConfigMapping mappingEntity : hostMappingsByType.get(entry.getKey())) {
@@ -2137,7 +2179,9 @@ public class ClusterImpl implements Cluster {
hostOverrides.add(new DesiredConfig.HostOverride(
hostIdToName.get(mappingEntity.getHostId()), mappingEntity.getVersion()));
}
- entry.getValue().setHostOverrides(hostOverrides);
+
+ for (DesiredConfig c: entry.getValue())
+ c.setHostOverrides(hostOverrides);
}
}
@@ -2148,6 +2192,7 @@ public class ClusterImpl implements Cluster {
}
+
@Override
public ServiceConfigVersionResponse createServiceConfigVersion(
String serviceName, String user, String note, ConfigGroup configGroup) {
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/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 608e6ca..d9ac183 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
@@ -63,11 +63,14 @@ import org.slf4j.LoggerFactory;
import javax.inject.Inject;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicLong;
@@ -429,17 +432,55 @@ public class AmbariContext {
}
}
- public boolean doesConfigurationWithTagExist(long clusterId, String tag) {
+ /**
+ * Verifies if the given cluster has at least one desired configuration transitioned through
+ * TopologyManager.INITIAL -> .... -> TopologyManager.TOPOLOGY_RESOLVED -> ....
+ * @param clusterId the identifier of the cluster to be checked
+ * @return true if the cluster
+ */
+ public boolean isTopologyResolved(long clusterId) {
boolean isTopologyResolved = false;
try {
Cluster cluster = getController().getClusters().getClusterById(clusterId);
- Collection<DesiredConfig> desiredConfigs = cluster.getDesiredConfigs().values();
- for (DesiredConfig config : desiredConfigs) {
- if (config.getTag().equals(tag)) {
+
+ // Check through the various cluster config versions that these transitioned through TopologyManager.INITIAL -> .... -> TopologyManager.TOPOLOGY_RESOLVED -> ....
+ Map<String, Set<DesiredConfig>> allDesiredConfigsByType = cluster.getAllDesiredConfigVersions();
+
+ for (String configType: allDesiredConfigsByType.keySet()) {
+ Set<DesiredConfig> desiredConfigVersions = allDesiredConfigsByType.get(configType);
+
+ SortedSet<DesiredConfig> desiredConfigsOrderedByVersion = new TreeSet<>(new Comparator<DesiredConfig>() {
+ @Override
+ public int compare(DesiredConfig o1, DesiredConfig o2) {
+ if (o1.getVersion() < o2.getVersion())
+ return -1;
+
+ if (o1.getVersion() > o2.getVersion())
+ return 1;
+
+ return 0;
+ }
+ });
+
+ desiredConfigsOrderedByVersion.addAll(desiredConfigVersions);
+
+ int tagMatchState = 0; // 0 -> INITIAL -> tagMatchState = 1 -> TOPLOGY_RESOLVED -> tagMatchState = 2
+
+ for (DesiredConfig config: desiredConfigsOrderedByVersion) {
+ if (config.getTag().equals(TopologyManager.INITIAL_CONFIG_TAG) && tagMatchState == 0)
+ tagMatchState = 1;
+ else if (config.getTag().equals(TopologyManager.TOPOLOGY_RESOLVED_TAG) && tagMatchState == 1) {
+ tagMatchState = 2;
+ break;
+ }
+ }
+
+ if (tagMatchState == 2) {
isTopologyResolved = true;
break;
}
}
+
} catch (ClusterNotFoundException e) {
LOG.info("Attempted to determine if configuration is topology resolved for a non-existent cluster: {}",
clusterId);
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java
index 9b6c9ad..960915f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java
@@ -640,7 +640,7 @@ public class TopologyManager {
if (!configChecked) {
configChecked = true;
- if (!ambariContext.doesConfigurationWithTagExist(topology.getClusterId(), TOPOLOGY_RESOLVED_TAG)) {
+ if (!ambariContext.isTopologyResolved(topology.getClusterId())) {
LOG.info("TopologyManager.replayRequests: no config with TOPOLOGY_RESOLVED found, adding cluster config request");
addClusterConfigRequest(topology, new ClusterConfigurationRequest(
ambariContext, topology, false, stackAdvisorBlueprintProcessor));
@@ -683,7 +683,7 @@ public class TopologyManager {
CLUSTER_CONFIG_TASK_MAX_TIME_IN_MILLIS_PROPERTY_NAME);
long timeout = 1000 * 60 * 30; // 30 minutes
- long delay = 100; //ms
+ long delay = 1000; //ms
if (timeoutStr != null) {
timeout = Long.parseLong(timeoutStr);
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/ambari-server/src/test/java/org/apache/ambari/server/state/DesiredConfigTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/DesiredConfigTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/DesiredConfigTest.java
index 93e3f07..e144e18 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/state/DesiredConfigTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/state/DesiredConfigTest.java
@@ -20,6 +20,8 @@ package org.apache.ambari.server.state;
import java.util.Arrays;
import java.util.List;
+import nl.jqno.equalsverifier.EqualsVerifier;
+import nl.jqno.equalsverifier.Warning;
import org.junit.Assert;
import org.junit.Test;
@@ -60,4 +62,19 @@ public class DesiredConfigTest {
Assert.assertEquals("Expected override version 'v1'", "v1", override.getVersionTag());
}
+ @Test
+ public void testEquals() throws Exception {
+ EqualsVerifier.forClass(DesiredConfig.class)
+ .usingGetClass()
+ .suppress(Warning.NONFINAL_FIELDS)
+ .verify();
+ }
+
+ @Test
+ public void testHostOverride_Equals() throws Exception {
+ EqualsVerifier.forClass(DesiredConfig.HostOverride.class)
+ .usingGetClass()
+ .verify();
+ }
+
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/4f0acbf1/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 254d3a3..1613d11 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
@@ -18,6 +18,8 @@
package org.apache.ambari.server.topology;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import org.apache.ambari.server.controller.AmbariManagementController;
import org.apache.ambari.server.controller.ClusterRequest;
import org.apache.ambari.server.controller.ConfigGroupRequest;
@@ -68,6 +70,7 @@ import static org.easymock.EasyMock.reset;
import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
/**
* AmbariContext unit tests
@@ -427,5 +430,109 @@ public class AmbariContextTest {
context.waitForConfigurationResolution(CLUSTER_NAME, testUpdatedConfigTypes);
}
+ @Test
+ public void testIsTopologyResolved_True() throws Exception {
+
+ // Given
+ DesiredConfig testHdfsDesiredConfig1 = new DesiredConfig();
+ testHdfsDesiredConfig1.setTag(TopologyManager.INITIAL_CONFIG_TAG);
+ testHdfsDesiredConfig1.setVersion(1L);
+
+ DesiredConfig testHdfsDesiredConfig2 = new DesiredConfig();
+ testHdfsDesiredConfig2.setTag(TopologyManager.TOPOLOGY_RESOLVED_TAG);
+ testHdfsDesiredConfig2.setVersion(2L);
+
+ DesiredConfig testHdfsDesiredConfig3 = new DesiredConfig();
+ testHdfsDesiredConfig3.setTag("ver123");
+ testHdfsDesiredConfig3.setVersion(3L);
+
+ DesiredConfig testCoreSiteDesiredConfig = new DesiredConfig();
+ testCoreSiteDesiredConfig.setTag("ver123");
+ testCoreSiteDesiredConfig.setVersion(1L);
+
+
+ Map<String, Set<DesiredConfig>> testDesiredConfigs = ImmutableMap.<String, Set<DesiredConfig>>builder()
+ .put("hdfs-site", ImmutableSet.of(testHdfsDesiredConfig2, testHdfsDesiredConfig3, testHdfsDesiredConfig1))
+ .put("core-site", ImmutableSet.of(testCoreSiteDesiredConfig))
+ .build();
+
+ expect(cluster.getAllDesiredConfigVersions()).andReturn(testDesiredConfigs).atLeastOnce();
+
+ replayAll();
+
+ // When
+ boolean topologyResolved = context.isTopologyResolved(CLUSTER_ID);
+
+ // Then
+ assertTrue(topologyResolved);
+ }
+
+ @Test
+ public void testIsTopologyResolved_WrongOrder_False() throws Exception {
+
+ // Given
+ DesiredConfig testHdfsDesiredConfig1 = new DesiredConfig();
+ testHdfsDesiredConfig1.setTag(TopologyManager.INITIAL_CONFIG_TAG);
+ testHdfsDesiredConfig1.setVersion(2L);
+
+ DesiredConfig testHdfsDesiredConfig2 = new DesiredConfig();
+ testHdfsDesiredConfig2.setTag(TopologyManager.TOPOLOGY_RESOLVED_TAG);
+ testHdfsDesiredConfig2.setVersion(1L);
+
+ DesiredConfig testHdfsDesiredConfig3 = new DesiredConfig();
+ testHdfsDesiredConfig3.setTag("ver123");
+ testHdfsDesiredConfig3.setVersion(3L);
+
+ DesiredConfig testCoreSiteDesiredConfig = new DesiredConfig();
+ testCoreSiteDesiredConfig.setTag("ver123");
+ testCoreSiteDesiredConfig.setVersion(1L);
+
+
+ Map<String, Set<DesiredConfig>> testDesiredConfigs = ImmutableMap.<String, Set<DesiredConfig>>builder()
+ .put("hdfs-site", ImmutableSet.of(testHdfsDesiredConfig2, testHdfsDesiredConfig3, testHdfsDesiredConfig1))
+ .put("core-site", ImmutableSet.of(testCoreSiteDesiredConfig))
+ .build();
+
+ expect(cluster.getAllDesiredConfigVersions()).andReturn(testDesiredConfigs).atLeastOnce();
+
+ replayAll();
+
+ // When
+ boolean topologyResolved = context.isTopologyResolved(CLUSTER_ID);
+
+ // Then due to INITIAL -> TOPOLOGY_RESOLVED not honored
+ assertFalse(topologyResolved);
+ }
+
+ @Test
+ public void testIsTopologyResolved_False() throws Exception {
+
+ // Given
+ DesiredConfig testHdfsDesiredConfig1 = new DesiredConfig();
+ testHdfsDesiredConfig1.setTag("ver1222");
+ testHdfsDesiredConfig1.setVersion(1L);
+
+
+ DesiredConfig testCoreSiteDesiredConfig = new DesiredConfig();
+ testCoreSiteDesiredConfig.setTag("ver123");
+ testCoreSiteDesiredConfig.setVersion(1L);
+
+
+ Map<String, Set<DesiredConfig>> testDesiredConfigs = ImmutableMap.<String, Set<DesiredConfig>>builder()
+ .put("hdfs-site", ImmutableSet.of(testHdfsDesiredConfig1))
+ .put("core-site", ImmutableSet.of(testCoreSiteDesiredConfig))
+ .build();
+
+ expect(cluster.getAllDesiredConfigVersions()).andReturn(testDesiredConfigs).atLeastOnce();
+
+ replayAll();
+
+ // When
+ boolean topologyResolved = context.isTopologyResolved(CLUSTER_ID);
+
+ // Then
+ assertFalse(topologyResolved);
+ }
+
}