From 4f2388970a567d1bcd0225a0f0845d92ea7f6090 Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Fri, 22 Jul 2016 13:45:37 -0700 Subject: AMBARI-17863 : AMS - topN does not work when metric name has a wildcard specified. (avijayan) --- .../metrics/timeline/query/DefaultCondition.java | 28 +++++- .../metrics/timeline/query/TopNCondition.java | 9 +- .../metrics/timeline/TestPhoenixTransactSQL.java | 33 +++---- .../metrics/timeline/TopNConditionTest.java | 105 +++++++++++++++++++++ 4 files changed, 150 insertions(+), 25 deletions(-) create mode 100644 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/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 81528df797..0851e8f0f8 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 metricNames, List 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 metricNames) { + for (String name : metricNames) { + if (name.contains("%")) { + return true; + } + } + return false; + } + + protected static boolean hostNamesHaveWildcard(List hostnames) { + if (hostnames == null) + return false; + for (String name : hostnames) { + if (name.contains("%")) { + return true; + } + } + return false; + } } 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 fae1655e6c..f7060e0394 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 metricNames, List 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 metricNames, List 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() { 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 b4cee67256..a95655dd2d 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( 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 0000000000..511019152f --- /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 metricNames = new ArrayList<>(); + List 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)); + + } +} -- cgit v1.2.3