You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by am...@apache.org on 2018/07/03 13:16:19 UTC

[ambari] branch trunk updated: AMBARI-24234. Alerts Are Running For Components Which Are Not Install… (#1663)

This is an automated email from the ASF dual-hosted git repository.

amagyar pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 30b776f  AMBARI-24234. Alerts Are Running For Components Which Are Not Install… (#1663)
30b776f is described below

commit 30b776f3df2a15476322dba7aa39dc388082954b
Author: Attila Magyar <m....@gmail.com>
AuthorDate: Tue Jul 3 15:16:15 2018 +0200

    AMBARI-24234. Alerts Are Running For Components Which Are Not Install… (#1663)
    
    * AMBARI-24234. Alerts Are Running For Components Which Are Not Installed (amagyar)
    
    * AMBARI-24234. Alerts Are Running For Components Which Are Not Installed (amagyar)
---
 .../alerts/AlertDefinitionsUIUpdateListener.java   |  8 ++---
 .../org/apache/ambari/server/state/Cluster.java    |  3 ++
 .../org/apache/ambari/server/state/Clusters.java   |  4 +--
 .../ambari/server/state/alert/AlertDefinition.java | 38 ++++++++++++++++++++++
 .../server/state/alert/AlertDefinitionHash.java    | 11 +------
 .../ambari/server/state/cluster/ClusterImpl.java   | 13 +-------
 .../ambari/server/state/cluster/ClustersImpl.java  |  4 +--
 7 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertDefinitionsUIUpdateListener.java b/ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertDefinitionsUIUpdateListener.java
index 59ce006..79cc93a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertDefinitionsUIUpdateListener.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/events/listeners/alerts/AlertDefinitionsUIUpdateListener.java
@@ -22,7 +22,6 @@ import static org.apache.ambari.server.events.AlertDefinitionEventType.UPDATE;
 
 import java.util.Collections;
 import java.util.Map;
-import java.util.Set;
 
 import javax.inject.Provider;
 
@@ -137,13 +136,14 @@ public class AlertDefinitionsUIUpdateListener {
 
   private void handleSingleDefinitionChange(AlertDefinitionEventType eventType, AlertDefinition alertDefinition) throws AmbariException {
     LOG.info("{} alert definition '{}'", eventType, alertDefinition);
-    Set<String> hosts = helper.get().invalidateHosts(alertDefinition);
-    for (String hostName : hosts) {
+    Cluster cluster = clusters.get().getCluster(alertDefinition.getClusterId());
+    helper.get().invalidateHosts(alertDefinition); // do we need to invalidate, what's the purpose of this?
+    for (String hostName : alertDefinition.matchingHosts(clusters.get())) {
       alertDefinitionsHolder.provideAlertDefinitionAgentUpdateEvent(eventType, alertDefinition.getClusterId(),
           Collections.singletonMap(alertDefinition.getDefinitionId(), alertDefinition), hostName);
     }
     if (alertDefinition.getName().equals(AMBARI_STALE_ALERT_NAME)) {
-      for (Host host : clusters.get().getCluster(alertDefinition.getClusterId()).getHosts()) {
+      for (Host host : cluster.getHosts()) {
         alertDefinitionsHolder.provideStaleAlertDefinitionUpdateEvent(eventType, alertDefinition.getClusterId(),
             alertHelper.getWaitFactorMultiplier(alertDefinition), host.getHostName());
       }
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 686af5c..c201310 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
@@ -18,6 +18,8 @@
 
 package org.apache.ambari.server.state;
 
+import static java.util.stream.Collectors.toSet;
+
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -131,6 +133,7 @@ public interface Cluster {
    * @return collection of hosts that are associated with this cluster
    */
   Collection<Host> getHosts();
+  default Set<String> getHostNames() { return getHosts().stream().map(Host::getHostName).collect(toSet()); }
 
   /**
    * Get all of the hosts running the provided service and component.
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 0795098..29ac724 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
@@ -185,10 +185,8 @@ public interface Clusters {
    * Gets all the hosts associated with the cluster
    * @param clusterName The name of the cluster
    * @return <code>Map</code> containing host name and <code>Host</code>
-   * @throws AmbariException
    */
-  Map<String, Host> getHostsForCluster(String clusterName)
-      throws AmbariException;
+  Map<String, Host> getHostsForCluster(String clusterName);
 
   /**
    * Gets all the host Ids associated with the cluster
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java b/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
index e654328..413e35a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
@@ -17,11 +17,20 @@
  */
 package org.apache.ambari.server.state.alert;
 
+import static java.util.Collections.emptySet;
+import static org.apache.ambari.server.controller.RootComponent.AMBARI_AGENT;
+import static org.apache.ambari.server.controller.RootService.AMBARI;
+
 import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
+import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.state.Alert;
 import org.apache.ambari.server.state.AlertState;
+import org.apache.ambari.server.state.Cluster;
+import org.apache.ambari.server.state.Clusters;
+import org.apache.ambari.server.state.ServiceComponentHost;
 import org.apache.commons.lang.StringUtils;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -419,4 +428,33 @@ public class AlertDefinition {
   public String toString() {
     return name;
   }
+
+  /**
+   * Collect the host names from the cluster where the given alert is allowed to run.
+   * A host is able to run an alert if the service component of the alert is installed on that particular host.
+   * In case of AMBARI server alerts or AGGREGATE alerts, an empty host set is returned.
+   * @return matching host names
+   */
+  public Set<String> matchingHosts(Clusters clusters) throws AmbariException {
+    Cluster cluster = clusters.getCluster(clusterId);
+    if (source.getType() == SourceType.AGGREGATE) {
+      return emptySet();
+    }
+    if (AMBARI.name().equals(serviceName)) {
+      return AMBARI_AGENT.name().equals(componentName) ? cluster.getHostNames() : emptySet();
+    }
+    Set<String> matchingHosts = new HashSet<>();
+    for (String host : cluster.getHostNames()) {
+      for (ServiceComponentHost component : cluster.getServiceComponentHosts(host)) {
+        if (belongsTo(component)) {
+          matchingHosts.add(host);
+        }
+      }
+    }
+    return matchingHosts;
+  }
+
+  private boolean belongsTo(ServiceComponentHost component) {
+    return component.getServiceName().equals(serviceName) && component.getServiceComponentName().equals(componentName);
+  }
 }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java b/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java
index 2f5fe29..d9edf1c 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java
@@ -361,19 +361,10 @@ public class AlertDefinitionHash {
       return Collections.emptySet();
     }
 
-    Map<String, Host> hosts = null;
     String clusterName = cluster.getClusterName();
+    Map<String, Host> hosts = m_clusters.get().getHostsForCluster(clusterName);
     Set<String> affectedHosts = new HashSet<>();
 
-    try {
-      hosts = m_clusters.get().getHostsForCluster(clusterName);
-    } catch (AmbariException ambariException) {
-      LOG.error("Unable to lookup hosts for cluster named {}", clusterName,
-          ambariException);
-
-      return affectedHosts;
-    }
-
     String ambariServiceName = RootService.AMBARI.name();
     String agentComponentName = RootComponent.AMBARI_AGENT.name();
 
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 95fa9c8..821bdbc 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
@@ -2246,18 +2246,7 @@ public class ClusterImpl implements Cluster {
 
   @Override
   public Collection<Host> getHosts() {
-    Map<String, Host> hosts;
-
-    try {
-      //todo: why the hell does this method throw AmbariException???
-      //todo: this is ridiculous that I need to get hosts for this cluster from Clusters!!!
-      //todo: should I getHosts using the same logic as the other getHosts call?  At least that doesn't throw AmbariException.
-      hosts =  clusters.getHostsForCluster(clusterName);
-    } catch (AmbariException e) {
-      //todo: in what conditions is AmbariException thrown?
-      throw new RuntimeException("Unable to get hosts for cluster: " + clusterName, e);
-    }
-    return hosts == null ? Collections.emptyList() : hosts.values();
+    return clusters.getHostsForCluster(clusterName).values();
   }
 
   private ClusterHealthReport getClusterHealthReport(
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 1100e09..7a3069b 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
@@ -706,9 +706,7 @@ public class ClustersImpl implements Clusters {
   }
 
   @Override
-  public Map<String, Host> getHostsForCluster(String clusterName)
-      throws AmbariException {
-
+  public Map<String, Host> getHostsForCluster(String clusterName) {
     Map<String, Host> hosts = new HashMap<>();
     for (Host h : getClusterHostsMap().get(clusterName)) {
       hosts.put(h.getHostName(), h);