Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-27 Thread Sergey Shelukhin 

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

(Updated March 27, 2018, 7:31 p.m.)


Review request for hive and Prasanth_J.


Repository: hive-git


Description
---

.


Diffs (updated)
-

  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 5888eaa6fb 
  
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/5/

Changes: https://reviews.apache.org/r/66237/diff/4-5/


Testing
---


Thanks,

Sergey Shelukhin

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-26 Thread j . prasanth . j 


> On March 24, 2018, 1:34 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmPoolMetrics.java
> > Lines 159 (patched)
> > 
> >
> > 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__
> > 
> > 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?
> 
> Sergey Shelukhin wrote:
> I'm not sure what you mean. I just used session ID as a standard tag, 
> since there's nothing else to put in there.
> Note that metrics are emitted into Hadoop metrics and also codahale 
> (mostly for HS2 JMX).
> Only codahale one uses silly names (which seems to be a common pattern if 
> you see HS2 JMX), the tagged metrics from Hadoop metrics should all have the 
> same name.

make sense.. missed the part about handling codahale vs hadoop metrics which 
supports tags.


- Prasanth_J


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


On March 26, 2018, 9:30 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66237/
> ---
> 
> (Updated March 26, 2018, 9:30 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/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-26 Thread j . prasanth . j 

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


Ship it!




Ship It!

- Prasanth_J


On March 26, 2018, 9:30 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66237/
> ---
> 
> (Updated March 26, 2018, 9:30 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/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-26 Thread Sergey Shelukhin 


> On March 24, 2018, 1:34 a.m., Prasanth_J wrote:
> > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
> > Lines 304 (patched)
> > 
> >
> > why do we need to remove the gauge names? why not add a guage and 
> > forget about it.

Cause if the pools are removed, the gauges will just sit there. Why keep them?


> On March 24, 2018, 1:34 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmPoolMetrics.java
> > Lines 159 (patched)
> > 
> >
> > 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__
> > 
> > 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?

I'm not sure what you mean. I just used session ID as a standard tag, since 
there's nothing else to put in there.
Note that metrics are emitted into Hadoop metrics and also codahale (mostly for 
HS2 JMX).
Only codahale one uses silly names (which seems to be a common pattern if you 
see HS2 JMX), the tagged metrics from Hadoop metrics should all have the same 
name.


- Sergey


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


On March 26, 2018, 9:30 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66237/
> ---
> 
> (Updated March 26, 2018, 9:30 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/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-26 Thread Sergey Shelukhin 

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

(Updated March 26, 2018, 9:30 p.m.)


Review request for hive and Prasanth_J.


Repository: hive-git


Description
---

.


Diffs (updated)
-

  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/4/

Changes: https://reviews.apache.org/r/66237/diff/3-4/


Testing
---


Thanks,

Sergey Shelukhin

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-26 Thread Sergey Shelukhin 

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

(Updated March 26, 2018, 9:01 p.m.)


Review request for hive and Prasanth_J.


Repository: hive-git


Description
---

.


Diffs (updated)
-

  
llap-server/src/java/org/apache/hadoop/hive/llap/counters/WmFragmentCounters.java
 8287adb636 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
 8cd723d2e0 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapTaskReporter.java
 b05e0b9e43 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
 b484a13e48 


Diff: https://reviews.apache.org/r/66237/diff/3/

Changes: https://reviews.apache.org/r/66237/diff/2-3/


Testing
---


Thanks,

Sergey Shelukhin

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-23 Thread j . prasanth . j 

---
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)


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)


nit: whitespace in many places.



ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmPoolMetrics.java
Lines 159 (patched)


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__

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
> 
>

Re: Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-23 Thread Sergey Shelukhin 

---
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 (updated)
-

  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/

Changes: https://reviews.apache.org/r/66237/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin

Review Request 66237: HIVE-18971 add HS2 WM metrics for use in Grafana and such

2018-03-22 Thread Sergey Shelukhin 

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

Review request for hive and Prasanth_J.


Repository: hive-git


Description
---

.


Diffs
-

  
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/1/


Testing
---


Thanks,

Sergey Shelukhin