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




common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
Lines 304 (patched)
<https://reviews.apache.org/r/66237/#comment280432>

    why do we need to remove the gauge names? why not add a guage and forget 
about it.



ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java
Lines 103 (patched)
<https://reviews.apache.org/r/66237/#comment280433>

    nit: whitespace in many places.



ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmPoolMetrics.java
Lines 159 (patched)
<https://reviews.apache.org/r/66237/#comment280434>

    why isn't this tag alone be sufficient? With this tag we can get all 
metrics associated/registered under pool. right?
    
    instead of emitting metrics like
    WM_<poolname>_<metricname>
    
    why not build a wrapper around this getMetrics() which gets all pool names 
and set the tag. So we will have something like
    
    {
    "tag.SessionId": "6020e225-f36e-470b-a170-b18e69af6fc8",
    "tag.Poolname": "llap",
    "NumExecutors": 2,
    "NumSessions": 2
    }
    
    If you try to run 2 LLAP daemons on the same host, you would get 2 
different metrics with different SessionId. This looks similar to that except 
that only thing that changes here is poolName. Am I missing something?


- Prasanth_J


On March 23, 2018, 8:14 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66237/
> -----------------------------------------------------------
> 
> (Updated March 23, 2018, 8:14 p.m.)
> 
> 
> Review request for hive and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> .
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java 
> effe26b6b6 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 
> 88c513b8cd 
>   
> common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
>  a43b09db8c 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8d9b5a3194 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapRecordReader.java
>  3a2c19a3e6 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java 
> a52928cc7a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/QueryAllocationManager.java 
> 9885ce7221 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmPoolMetrics.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
> f0e620c684 
>   
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/TezProgressMonitor.java
>  a14cdb609a 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
> 20a5947291 
> 
> 
> Diff: https://reviews.apache.org/r/66237/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to