ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From smoha...@apache.org
Subject ambari git commit: AMBARI-14188. During Upgrade Topology Manager Causes Ambari To Be Unresponsive With Infinite Loop (Sebastian Toader via smohanty)
Date Mon, 07 Dec 2015 00:32:09 GMT
Repository: ambari
Updated Branches:
  refs/heads/branch-2.2 40cd4f405 -> 041d040d8


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/041d040d
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/041d040d
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/041d040d

Branch: refs/heads/branch-2.2
Commit: 041d040d805e1392b31ad9099982e96c00fa384a
Parents: 40cd4f4
Author: Sumit Mohanty <smohanty@hortonworks.com>
Authored: Sun Dec 6 16:06:35 2015 -0800
Committer: Sumit Mohanty <smohanty@hortonworks.com>
Committed: Sun Dec 6 16:31:54 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/041d040d/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 3aaa9a5..e10c808 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/041d040d/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/041d040d/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 a556db8..16f4120 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/041d040d/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/041d040d/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/041d040d/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/041d040d/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);
+  }
+
 
 }


Mime
View raw message