AMBARI-17863 : AMS - topN does not work when metric name has a wildcard 
specified. (avijayan)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/7155e8e7
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/7155e8e7
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/7155e8e7

Branch: refs/heads/trunk
Commit: 7155e8e7283eb837a6b6be648eb513cbcf7b1f26
Parents: 1a415a6
Author: Aravindan Vijayan <[email protected]>
Authored: Fri Jul 22 15:08:55 2016 -0700
Committer: Aravindan Vijayan <[email protected]>
Committed: Fri Jul 22 15:08:55 2016 -0700

----------------------------------------------------------------------
 .../timeline/query/DefaultCondition.java        |  28 ++++-
 .../metrics/timeline/query/TopNCondition.java   |   9 +-
 .../timeline/TestPhoenixTransactSQL.java        |  33 +++---
 .../metrics/timeline/TopNConditionTest.java     | 105 +++++++++++++++++++
 4 files changed, 150 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/7155e8e7/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/DefaultCondition.java
----------------------------------------------------------------------
diff --git 
a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/DefaultCondition.java
 
b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/DefaultCondition.java
index 81528df..0851e8f 100644
--- 
a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/DefaultCondition.java
+++ 
b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/DefaultCondition.java
@@ -44,8 +44,8 @@ public class DefaultCondition implements Condition {
   private static final Log LOG = LogFactory.getLog(DefaultCondition.class);
 
   public DefaultCondition(List<String> metricNames, List<String> hostnames, 
String appId,
-                   String instanceId, Long startTime, Long endTime, Precision 
precision,
-                   Integer limit, boolean grouped) {
+                          String instanceId, Long startTime, Long endTime, 
Precision precision,
+                          Integer limit, boolean grouped) {
     this.metricNames = metricNames;
     this.hostnames = hostnames;
     this.appId = appId;
@@ -112,7 +112,7 @@ public class DefaultCondition implements Condition {
 
   public String getAppId() {
     if (appId != null && !appId.isEmpty()) {
-      if (!(appId.equals("HOST") || appId.equals("FLUME_HANDLER")) ) {
+      if (!(appId.equals("HOST") || appId.equals("FLUME_HANDLER"))) {
         return appId.toLowerCase();
       } else {
         return appId;
@@ -233,7 +233,7 @@ public class DefaultCondition implements Condition {
         }
       }
 
-      if (metricsIn.length()>0) {
+      if (metricsIn.length() > 0) {
         sb.append("(METRIC_NAME IN (");
         sb.append(metricsIn);
         sb.append(")");
@@ -313,4 +313,24 @@ public class DefaultCondition implements Condition {
       ", noLimit=" + noLimit +
       '}';
   }
+
+  protected static boolean metricNamesHaveWildcard(List<String> metricNames) {
+    for (String name : metricNames) {
+      if (name.contains("%")) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  protected static boolean hostNamesHaveWildcard(List<String> hostnames) {
+    if (hostnames == null)
+      return false;
+    for (String name : hostnames) {
+      if (name.contains("%")) {
+        return true;
+      }
+    }
+    return false;
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/7155e8e7/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
----------------------------------------------------------------------
diff --git 
a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
 
b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
index fae1655..f7060e0 100644
--- 
a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
+++ 
b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
@@ -150,7 +150,9 @@ public class TopNCondition extends DefaultCondition{
   public static boolean isTopNHostCondition(List<String> metricNames, 
List<String> hostnames) {
     // Case 1 : 1 Metric, H hosts
     // Select Top N or Bottom N host series based on 1 metric (max/avg/sum)
-    return (metricNames.size() == 1 && CollectionUtils.isNotEmpty(hostnames));
+    // Hostnames cannot be empty
+    // Only 1 metric allowed, without wildcards
+    return (CollectionUtils.isNotEmpty(hostnames) && metricNames.size() == 1 
&& !metricNamesHaveWildcard(metricNames));
 
   }
 
@@ -163,7 +165,10 @@ public class TopNCondition extends DefaultCondition{
   public static boolean isTopNMetricCondition(List<String> metricNames, 
List<String> hostnames) {
     // Case 2 : M Metric names or Regex, 1 or No host
     // Select Top N or Bottom N metric series based on metric 
values(max/avg/sum)
-    return (metricNames.size() > 1 && (hostnames == null || hostnames.size() 
<= 1));
+    // MetricNames cannot be empty
+    // No host (aggregate) or 1 host allowed, without wildcards
+    return (CollectionUtils.isNotEmpty(metricNames) && (hostnames == null || 
hostnames.size() <= 1) &&
+      !hostNamesHaveWildcard(hostnames));
   }
 
   public Integer getTopN() {

http://git-wip-us.apache.org/repos/asf/ambari/blob/7155e8e7/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestPhoenixTransactSQL.java
----------------------------------------------------------------------
diff --git 
a/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestPhoenixTransactSQL.java
 
b/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestPhoenixTransactSQL.java
index b4cee67..a95655d 100644
--- 
a/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestPhoenixTransactSQL.java
+++ 
b/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestPhoenixTransactSQL.java
@@ -620,31 +620,26 @@ public class TestPhoenixTransactSQL {
             "a1", "i1", 1407959718L, 1407959918L, null, null, false, 2, null, 
false);
 
     String conditionClause = condition.getConditionClause().toString();
-    String expectedClause = "(METRIC_NAME IN (?)) AND HOSTNAME IN (" +
-            "SELECT " + 
PhoenixTransactSQL.getNaiveTimeRangeHint(condition.getStartTime(),120000l) +
-            " HOSTNAME FROM METRIC_RECORD WHERE " +
-            "(METRIC_NAME IN (?)) AND " +
-            "(HOSTNAME LIKE ? OR HOSTNAME LIKE ?) AND " +
-            "APP_ID = ? AND INSTANCE_ID = ? AND " +
-            "SERVER_TIME >= ? AND SERVER_TIME < ? " +
-            "GROUP BY METRIC_NAME, HOSTNAME, APP_ID ORDER BY MAX(METRIC_MAX) 
DESC LIMIT 2) " +
-            "AND APP_ID = ? AND INSTANCE_ID = ? AND SERVER_TIME >= ? AND 
SERVER_TIME < ?";
+    String expectedClause = "(METRIC_NAME IN (?)) AND HOSTNAME IN (SELECT " +
+      
PhoenixTransactSQL.getNaiveTimeRangeHint(condition.getStartTime(),120000l) +
+      " HOSTNAME FROM METRIC_RECORD WHERE (METRIC_NAME IN (?)) " +
+      "AND (HOSTNAME LIKE ? OR HOSTNAME LIKE ?) AND APP_ID = ? AND INSTANCE_ID 
= ? AND SERVER_TIME >= ? AND SERVER_TIME < ? GROUP BY " +
+      "METRIC_NAME, HOSTNAME, APP_ID ORDER BY MAX(METRIC_MAX) DESC LIMIT 2) 
AND APP_ID = ? AND INSTANCE_ID = ? AND " +
+      "SERVER_TIME >= ? AND SERVER_TIME < ?";
     Assert.assertEquals(expectedClause, conditionClause);
 
     condition = new TopNCondition(
-            Arrays.asList("m1", "m2", "m3"), Arrays.asList("%.ambari"),
+            Arrays.asList("m1"), Arrays.asList("%.ambari"),
             "a1", "i1", 1407959718L, 1407959918L, null, null, false, 2, null, 
false);
 
     conditionClause = condition.getConditionClause().toString();
-    expectedClause = " METRIC_NAME IN (" +
-            "SELECT " + 
PhoenixTransactSQL.getNaiveTimeRangeHint(condition.getStartTime(),120000l) +
-            " METRIC_NAME FROM METRIC_RECORD WHERE " +
-            "(METRIC_NAME IN (?, ?, ?)) AND " +
-            "(HOSTNAME LIKE ?) AND " +
-            "APP_ID = ? AND INSTANCE_ID = ? AND " +
-            "SERVER_TIME >= ? AND SERVER_TIME < ? " +
-            "GROUP BY METRIC_NAME, APP_ID ORDER BY MAX(METRIC_MAX) DESC LIMIT 
2) " +
-            "AND (HOSTNAME LIKE ?) AND APP_ID = ? AND INSTANCE_ID = ? AND 
SERVER_TIME >= ? AND SERVER_TIME < ?";
+    expectedClause = "(METRIC_NAME IN (?)) AND HOSTNAME IN (SELECT " +
+      
PhoenixTransactSQL.getNaiveTimeRangeHint(condition.getStartTime(),120000l) +
+      " HOSTNAME FROM METRIC_RECORD WHERE (METRIC_NAME IN (?)) " +
+      "AND (HOSTNAME LIKE ?) AND APP_ID = ? AND INSTANCE_ID = ? AND 
SERVER_TIME >= ? AND SERVER_TIME < ? GROUP BY " +
+      "METRIC_NAME, HOSTNAME, APP_ID ORDER BY MAX(METRIC_MAX) DESC LIMIT 2) 
AND APP_ID = ? AND INSTANCE_ID = ? AND " +
+      "SERVER_TIME >= ? AND SERVER_TIME < ?";
+
     Assert.assertEquals(expectedClause, conditionClause);
 
     condition = new TopNCondition(

http://git-wip-us.apache.org/repos/asf/ambari/blob/7155e8e7/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TopNConditionTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TopNConditionTest.java
 
b/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TopNConditionTest.java
new file mode 100644
index 0000000..5110191
--- /dev/null
+++ 
b/ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TopNConditionTest.java
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package 
org.apache.hadoop.yarn.server.applicationhistoryservice.metrics.timeline;
+
+import junit.framework.Assert;
+import 
org.apache.hadoop.yarn.server.applicationhistoryservice.metrics.timeline.query.TopNCondition;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class TopNConditionTest {
+
+
+  @Test
+  public void testTopNCondition() {
+
+    List<String> metricNames = new ArrayList<>();
+    List<String> hostnames = new ArrayList<>();
+
+    //Valid Cases
+
+    // "H" hosts and 1 Metric
+    hostnames.add("h1");
+    hostnames.add("h2");
+    metricNames.add("m1");
+    Assert.assertTrue(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertFalse(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+    hostnames.clear();
+
+    // Host(s) with wildcard & 1 Metric
+    hostnames.add("h%");
+    hostnames.add("g1");
+    Assert.assertTrue(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertFalse(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+    hostnames.clear();
+
+    // M Metrics and No host
+    metricNames.add("m2");
+    metricNames.add("m3");
+    Assert.assertFalse(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertTrue(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+
+    // M Metrics with wildcard and No host
+    metricNames.add("m2");
+    metricNames.add("m%");
+    Assert.assertFalse(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertTrue(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+
+    // M Metrics with wildcard and 1 host
+    metricNames.add("m2");
+    metricNames.add("m%");
+    hostnames.add("h1");
+    Assert.assertFalse(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertTrue(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+
+    metricNames.clear();
+    hostnames.clear();
+
+    //Invalid Cases
+    // M metrics and H hosts
+    metricNames.add("m1");
+    metricNames.add("m2");
+    hostnames.add("h1");
+    hostnames.add("h2");
+    Assert.assertFalse(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertFalse(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+
+    metricNames.clear();
+    hostnames.clear();
+
+    // Wildcard in 1 and multiple in other
+    metricNames.add("m1");
+    metricNames.add("m2");
+    hostnames.add("%");
+    Assert.assertFalse(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertFalse(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+
+    metricNames.clear();
+    hostnames.clear();
+
+    //Wildcard in both
+    metricNames.add("m%");
+    hostnames.add("%");
+    Assert.assertFalse(TopNCondition.isTopNHostCondition(metricNames, 
hostnames));
+    Assert.assertFalse(TopNCondition.isTopNMetricCondition(metricNames, 
hostnames));
+
+  }
+}

Reply via email to