-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47142/#review132356
-----------------------------------------------------------


Fix it, then Ship it!





ambari-metrics/ambari-metrics-common/src/main/java/org/apache/hadoop/metrics2/sink/timeline/TopNConfig.java
 (line 21)
<https://reviews.apache.org/r/47142/#comment196529>

    Missing JSON annotations.



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java
 (line 218)
<https://reviews.apache.org/r/47142/#comment196530>

    Lets make this configurable, no need to add config to the xml.



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java
 (line 308)
<https://reviews.apache.org/r/47142/#comment196535>

    We should cleanup the interface and remove this sigular function call 
unless it is documented as supported API.



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStore.java
 (line 53)
<https://reviews.apache.org/r/47142/#comment196540>

    formatting



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/PhoenixTransactSQL.java
 (line 472)
<https://reviews.apache.org/r/47142/#comment196541>

    Can we do refactor this? Builder would be great.



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/PhoenixTransactSQL.java
 (line 872)
<https://reviews.apache.org/r/47142/#comment196542>

    Rename to getTargetTableUsingPrecision



ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
 (line 146)
<https://reviews.apache.org/r/47142/#comment196543>

    Add javadoc for method


- Sid Wagle


On May 9, 2016, 11:50 p.m., Aravindan Vijayan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47142/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 11:50 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen, Sumit Mohanty, and Sid Wagle.
> 
> 
> Bugs: AMBARI-16412
>     https://issues.apache.org/jira/browse/AMBARI-16412
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Adding TopN support to AMS API
> 
> Following are supported.
> 
> 1. Top "N" hosts when 1 metric is requested for "H" hosts.
> 2. Top "N" metrics when "M" metrics (or a pattern) are requested for 0 or 1 
> hosts.
> 3. When more than 20 hosts are requested, AMS defaults to Top N hosts 
> condition with N=20.
> 
> 
> Diffs
> -----
> 
>   
> ambari-metrics/ambari-metrics-common/src/main/java/org/apache/hadoop/metrics2/sink/timeline/TopNConfig.java
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/HBaseTimelineMetricStore.java
>  89c67d1 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/PhoenixHBaseAccessor.java
>  a0ecbcc 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStore.java
>  ded64e3 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStoreWatcher.java
>  632df3f 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/ConditionBuilder.java
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/DefaultCondition.java
>  99a6125 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/PhoenixTransactSQL.java
>  6ee0006 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/query/TopNCondition.java
>  PRE-CREATION 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TimelineWebServices.java
>  4cfc415 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestPhoenixTransactSQL.java
>  9c6617c 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TestTimelineMetricStore.java
>  16bbf0e 
>   
> ambari-metrics/ambari-metrics-timelineservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/metrics/timeline/TimelineMetricStoreWatcherTest.java
>  277a98c 
> 
> Diff: https://reviews.apache.org/r/47142/diff/
> 
> 
> Testing
> -------
> 
> Manually tested.
> Unit tests pass.
> 
> 
> Thanks,
> 
> Aravindan Vijayan
> 
>

Reply via email to