Revert "AMBARI-9651 Add API level validation and error handling to AMS timeline service (dsen)"
This reverts commit 9429e62f8a972275469bf98b792d65f261d8737d. Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/0fde880b Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/0fde880b Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/0fde880b Branch: refs/heads/trunk Commit: 0fde880b8d4a0a8c659c1f9099f77ec0f08f252f Parents: 5392227 Author: Dmytro Sen <d...@apache.org> Authored: Thu Feb 19 12:03:20 2015 +0200 Committer: Dmytro Sen <d...@apache.org> Committed: Thu Feb 19 18:50:57 2015 +0200 ---------------------------------------------------------------------- .../timeline/HBaseTimelineMetricStore.java | 28 ---------- .../metrics/timeline/PhoenixHBaseAccessor.java | 11 ++-- .../metrics/timeline/PhoenixTransactSQL.java | 55 +++++++------------- .../webapp/TimelineWebServices.java | 48 ++++------------- .../0.1.0/package/scripts/service_check.py | 3 +- 5 files changed, 32 insertions(+), 113 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/0fde880b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java ---------------------------------------------------------------------- diff --git a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java index 0833a04..88086d6 100644 --- a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java +++ b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java @@ -109,20 +109,6 @@ 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); @@ -219,20 +205,6 @@ 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)); http://git-wip-us.apache.org/repos/asf/ambari/blob/0fde880b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java ---------------------------------------------------------------------- diff --git a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java index af05d93..6ffdc38 100644 --- a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java +++ b/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 { - validateConditionIsNotEmpty(condition); + verifyCondition(condition); Connection conn = getConnection(); PreparedStatement stmt = null; @@ -481,9 +481,6 @@ 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); @@ -515,7 +512,7 @@ public class PhoenixHBaseAccessor { Map<String, List<Function>> metricFunctions) throws SQLException { - validateConditionIsNotEmpty(condition); + verifyCondition(condition); Connection conn = getConnection(); PreparedStatement stmt = null; @@ -685,9 +682,9 @@ public class PhoenixHBaseAccessor { return metric; } - private void validateConditionIsNotEmpty(Condition condition) { + private void verifyCondition(Condition condition) throws SQLException { if (condition.isEmpty()) { - throw new IllegalArgumentException("No filter criteria specified."); + throw new SQLException("No filter criteria specified."); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/0fde880b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixTransactSQL.java ---------------------------------------------------------------------- diff --git a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixTransactSQL.java b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixTransactSQL.java index 80c6545..c3f231f 100644 --- a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixTransactSQL.java +++ b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixTransactSQL.java @@ -26,7 +26,6 @@ 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. @@ -232,9 +231,9 @@ public class PhoenixTransactSQL { public static PreparedStatement prepareGetMetricsSqlStmt( Connection connection, Condition condition) throws SQLException { - validateConditionIsNotEmpty(condition); - validateRowCountLimit(condition); - + if (condition.isEmpty()) { + throw new IllegalArgumentException("Condition is empty."); + } String stmtStr; if (condition.getStatement() != null) { stmtStr = condition.getStatement(); @@ -344,41 +343,13 @@ 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 { - validateConditionIsNotEmpty(condition); + if (condition.isEmpty()) { + throw new IllegalArgumentException("Condition is empty."); + } if (condition.getMetricNames() == null || condition.getMetricNames().size() == 0) { @@ -450,7 +421,9 @@ public class PhoenixTransactSQL { public static PreparedStatement prepareGetAggregateSqlStmt( Connection connection, Condition condition) throws SQLException { - validateConditionIsNotEmpty(condition); + if (condition.isEmpty()) { + throw new IllegalArgumentException("Condition is empty."); + } String metricsAggregateTable; String queryStmt; @@ -520,7 +493,15 @@ public class PhoenixTransactSQL { public static PreparedStatement prepareGetLatestAggregateMetricSqlStmt( Connection connection, Condition condition) throws SQLException { - validateConditionIsNotEmpty(condition); + 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 "); + } String stmtStr; if (condition.getStatement() != null) { http://git-wip-us.apache.org/repos/asf/ambari/blob/0fde880b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java ---------------------------------------------------------------------- diff --git a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java index bf7cefb..d6d637f 100644 --- a/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java +++ b/ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java @@ -24,7 +24,6 @@ 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; @@ -302,24 +301,15 @@ 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, endTime and limit should be " + - "numeric values"); + throw new BadRequestException("startTime 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); @@ -362,13 +352,11 @@ public class TimelineWebServices { ) { init(res); try { - if (LOG.isDebugEnabled()) { - LOG.debug("Request for metrics => metricNames: " + metricNames + ", " + - "appId: " + appId + ", instanceId: " + instanceId + ", " + - "hostname: " + hostname + ", startTime: " + startTime + ", " + - "endTime: " + endTime + ", " + - "precision: " + precision); - } + 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, @@ -382,8 +370,6 @@ 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); @@ -518,28 +504,12 @@ 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) { - 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; + return str == null ? null : Arrays.asList(str.trim().split(delimiter)); } private static String parseStr(String str) { return str == null ? null : str.trim(); } + } http://git-wip-us.apache.org/repos/asf/ambari/blob/0fde880b/ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/service_check.py ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/service_check.py b/ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/service_check.py index fdf1fab..b5a5745 100644 --- a/ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/package/scripts/service_check.py +++ b/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 = int(time.time())*1000 + current_time = time.time() 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,7 +112,6 @@ class AMSServiceCheck(Script): "appId": "amssmoketestfake", "hostname": params.hostname, "startTime": 1419860000000, - "endTime": int(time.time())*1000+1000, "precision": "seconds", "grouped": "false", }
