AMBARI-9651 Add API level validation and error handling to AMS timeline service (dsen)

ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java

@@ -109,6 +109,20 @@ public class HBaseTimelineMetricStore extends AbstractService
       Long startTime, Long endTime, Precision precision, Integer limit,
       boolean groupedByHosts) throws SQLException, IOException {
 
+
+    if (metricNames == null || metricNames.isEmpty()) {
+      throw new IllegalArgumentException("No metric name filter specified.");
+    }
+    if (applicationId == null || applicationId.trim().length() == 0) {
+      throw new IllegalArgumentException("No applicationID filter specified.");
+    }
+    if ((startTime == null && endTime != null)
+      ||(startTime != null && endTime == null)) {
+      throw new IllegalArgumentException("Open ended query not supported ");
+    }
+    if (limit != null && limit > PhoenixHBaseAccessor.RESULTSET_LIMIT){
+      throw new IllegalArgumentException("Limit too big");
+    }
     Map<String, List<Function>> metricFunctions =
       parseMetricNamesToAggregationFunctions(metricNames);
 
@@ -205,6 +219,20 @@ public class HBaseTimelineMetricStore extends AbstractService
       Long endTime, Precision precision, Integer limit)
       throws SQLException, IOException {
 
+    if (metricName == null || metricName.isEmpty()) {
+      throw new IllegalArgumentException("No metric name filter specified.");
+    }
+    if (applicationId == null || applicationId.trim().length() == 0) {
+      throw new IllegalArgumentException("No applicationID filter specified.");
+    }
+    if ((startTime == null && endTime != null)
+      ||(startTime != null && endTime == null)) {
+      throw new IllegalArgumentException("Open ended query not supported ");
+    }
+    if (limit !=null && limit > PhoenixHBaseAccessor.RESULTSET_LIMIT){
+      throw new IllegalArgumentException("Limit too big");
+    }
+
     Map<String, List<Function>> metricFunctions =
       parseMetricNamesToAggregationFunctions(Collections.singletonList(metricName));
 

ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java

@@ -404,7 +404,7 @@ public class PhoenixHBaseAccessor {
     final Condition condition, Map<String, List<Function>> metricFunctions)
     throws SQLException, IOException {
 
-    verifyCondition(condition);
+    validateConditionIsNotEmpty(condition);
 
     Connection conn = getConnection();
     PreparedStatement stmt = null;
@@ -481,6 +481,9 @@ public class PhoenixHBaseAccessor {
   private PreparedStatement getLatestMetricRecords(
     Condition condition, Connection conn, TimelineMetrics metrics)
     throws SQLException, IOException {
+
+    validateConditionIsNotEmpty(condition);
+
     PreparedStatement stmt = null;
     SplitByMetricNamesCondition splitCondition =
       new SplitByMetricNamesCondition(condition);
@@ -512,7 +515,7 @@ public class PhoenixHBaseAccessor {
     Map<String, List<Function>> metricFunctions)
     throws SQLException {
 
-    verifyCondition(condition);
+    validateConditionIsNotEmpty(condition);
 
     Connection conn = getConnection();
     PreparedStatement stmt = null;
@@ -682,9 +685,9 @@ public class PhoenixHBaseAccessor {
     return metric;
   }
 
-  private void verifyCondition(Condition condition) throws SQLException {
+  private void validateConditionIsNotEmpty(Condition condition) {
     if (condition.isEmpty()) {
-      throw new SQLException("No filter criteria specified.");
+      throw new IllegalArgumentException("No filter criteria specified.");
     }
   }
 

ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixTransactSQL.java

@@ -26,6 +26,7 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Encapsulate all metrics related SQL queries.
@@ -231,9 +232,9 @@ public class PhoenixTransactSQL {
   public static PreparedStatement prepareGetMetricsSqlStmt(
     Connection connection, Condition condition) throws SQLException {
 
-    if (condition.isEmpty()) {
-      throw new IllegalArgumentException("Condition is empty.");
-    }
+    validateConditionIsNotEmpty(condition);
+    validateRowCountLimit(condition);
+
     String stmtStr;
     if (condition.getStatement() != null) {
       stmtStr = condition.getStatement();
@@ -343,13 +344,41 @@ public class PhoenixTransactSQL {
     return stmt;
   }
 
+  private static void validateConditionIsNotEmpty(Condition condition) {
+    if (condition.isEmpty()) {
+      throw new IllegalArgumentException("Condition is empty.");
+    }
+  }
+
+  private static void validateRowCountLimit(Condition condition) {
+    if (condition.getMetricNames() == null
+      || condition.getMetricNames().size() ==0 ) {
+      //aggregator can use empty metrics query
+      return;
+    }
+
+    long range = condition.getEndTime() - condition.getStartTime();
+    long rowsPerMetric = TimeUnit.MILLISECONDS.toHours(range) + 1;
+
+    Precision precision = condition.getPrecision();
+    // for minutes and seconds we can use the rowsPerMetric computed based on
+    // minutes
+    if (precision != null && precision == Precision.HOURS) {
+      rowsPerMetric = TimeUnit.MILLISECONDS.toHours(range) + 1;
+    }
+
+    long totalRowsRequested = rowsPerMetric * condition.getMetricNames().size();
+    if (totalRowsRequested > PhoenixHBaseAccessor.RESULTSET_LIMIT) {
+      throw new IllegalArgumentException("The time range query for " +
+        "precision table exceeds row count limit, please query aggregate " +
+        "table instead.");
+    }
+  }
 
   public static PreparedStatement prepareGetLatestMetricSqlStmt(
     Connection connection, Condition condition) throws SQLException {
 
-    if (condition.isEmpty()) {
-      throw new IllegalArgumentException("Condition is empty.");
-    }
+    validateConditionIsNotEmpty(condition);
 
     if (condition.getMetricNames() == null
       || condition.getMetricNames().size() == 0) {
@@ -421,9 +450,7 @@ public class PhoenixTransactSQL {
   public static PreparedStatement prepareGetAggregateSqlStmt(
     Connection connection, Condition condition) throws SQLException {
 
-    if (condition.isEmpty()) {
-      throw new IllegalArgumentException("Condition is empty.");
-    }
+    validateConditionIsNotEmpty(condition);
 
     String metricsAggregateTable;
     String queryStmt;
@@ -493,15 +520,7 @@ public class PhoenixTransactSQL {
   public static PreparedStatement prepareGetLatestAggregateMetricSqlStmt(
     Connection connection, Condition condition) throws SQLException {
 
-    if (condition.isEmpty()) {
-      throw new IllegalArgumentException("Condition is empty.");
-    }
-
-    if (condition.getMetricNames() == null
-      || condition.getMetricNames().size() == 0) {
-      throw new IllegalArgumentException("Point in time query without " +
-        "metric names not supported ");
-    }
+    validateConditionIsNotEmpty(condition);
 
     String stmtStr;
     if (condition.getStatement() != null) {

ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java

@@ -24,6 +24,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Unstable;
+import org.apache.hadoop.util.StringUtils;
 import org.apache.hadoop.yarn.api.records.timeline.TimelineEntities;
 import org.apache.hadoop.yarn.api.records.timeline.TimelineEntity;
 import org.apache.hadoop.yarn.api.records.timeline.TimelineEvents;
@@ -301,15 +302,24 @@ public class TimelineWebServices {
   ) {
     init(res);
     try {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Request for metrics => metricName: " + metricName + ", " +
+          "appId: " + appId + ", instanceId: " + instanceId + ", " +
+          "hostname: " + hostname + ", startTime: " + startTime + ", " +
+          "endTime: " + endTime);
+      }
+
       return timelineMetricStore.getTimelineMetric(metricName, hostname,
         appId, instanceId, parseLongStr(startTime), parseLongStr(endTime),
         Precision.getPrecision(precision), parseIntStr(limit));
     } catch (NumberFormatException ne) {
-      throw new BadRequestException("startTime and limit should be numeric " +
-        "values");
+      throw new BadRequestException("startTime, endTime and limit should be " +
+        "numeric values");
     } catch (Precision.PrecisionFormatException pfe) {
       throw new BadRequestException("precision should be seconds, minutes " +
         "or hours");
+    } catch (IllegalArgumentException iae) {
+      throw new BadRequestException(iae.getMessage());
     } catch (SQLException sql) {
       throw new WebApplicationException(sql,
         Response.Status.INTERNAL_SERVER_ERROR);
@@ -352,11 +362,13 @@ public class TimelineWebServices {
   ) {
     init(res);
     try {
-      LOG.debug("Request for metrics => metricNames: " + metricNames + ", " +
-        "appId: " + appId + ", instanceId: " + instanceId + ", " +
-        "hostname: " + hostname + ", startTime: " + startTime + ", " +
-        "endTime: " + endTime + ", " +
-        "precision: " + precision);
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Request for metrics => metricNames: " + metricNames + ", " +
+          "appId: " + appId + ", instanceId: " + instanceId + ", " +
+          "hostname: " + hostname + ", startTime: " + startTime + ", " +
+          "endTime: " + endTime + ", " +
+          "precision: " + precision);
+      }
 
       return timelineMetricStore.getTimelineMetrics(
         parseListStr(metricNames, ","), hostname, appId, instanceId,
@@ -370,6 +382,8 @@ public class TimelineWebServices {
     } catch (Precision.PrecisionFormatException pfe) {
       throw new BadRequestException("precision should be seconds, minutes " +
         "or hours");
+    } catch (IllegalArgumentException iae) {
+      throw new BadRequestException(iae.getMessage());
     } catch (SQLException sql) {
       throw new WebApplicationException(sql,
         Response.Status.INTERNAL_SERVER_ERROR);
@@ -504,12 +518,28 @@ public class TimelineWebServices {
     return booleanStr == null || Boolean.parseBoolean(booleanStr);
   }
 
+  /**
+   * Parses delimited string to list of strings. It skips strings that are
+   * effectively empty (i.e. only whitespace).
+   *
+   */
   private static List<String> parseListStr(String str, String delimiter) {
-    return str == null ? null : Arrays.asList(str.trim().split(delimiter));
+    if (str == null || str.trim().isEmpty()){
+      return null;
+    }
+
+    String[] split = str.trim().split(delimiter);
+    List<String> list = new ArrayList<String>(split.length);
+    for (String s : split) {
+      if (!s.trim().isEmpty()){
+        list.add(s);
+      }
+    }
+
+    return list;
   }
 
   private static String parseStr(String str) {
     return str == null ? null : str.trim();
   }
-
 }

ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/service_check.py

@@ -68,7 +68,7 @@ class AMSServiceCheck(Script):
     env.set_params(params)
 
     random_value1 = random.random()
-    current_time = time.time()
+    current_time = int(time.time())*1000
     metric_json = Template('smoketest_metrics.json.j2', hostname=params.hostname, random1=random_value1,
                            current_time=current_time).get_content()
     Logger.info("Generated metrics:\n%s" % metric_json)
@@ -112,6 +112,7 @@ class AMSServiceCheck(Script):
       "appId": "amssmoketestfake",
       "hostname": params.hostname,
       "startTime": 1419860000000,
+      "endTime": int(time.time())*1000+1000,
       "precision": "seconds",
       "grouped": "false",
     }