You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Barna Zsombor Klara <zs...@cloudera.com> on 2016/10/10 12:16:06 UTC

Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

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

Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.


Repository: hive-git


Description
-------

HIVE-14754: Track the queries execution lifecycle times
Five new metrics are being added to track the query execution in HiveServer2. 
- hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
- hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
- hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
- hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 5/10/15 minutes 
- hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 5/10/15 minutes


Diffs
-----

  common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
  common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
  common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
  service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
  service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 

Diff: https://reviews.apache.org/r/52684/diff/


Testing
-------

Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
Unit tests added for the new MetricsQueryLifeTimeHook class.
Manually tested the new metrics by executing queries in HS2.


Thanks,

Barna Zsombor Klara


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Marta Kuczora <ku...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52684/#review152642
-----------------------------------------------------------



Thanks a lot for the patch, it very nice and clean! 
It looks good to me.

- Marta Kuczora


On Oct. 10, 2016, 5:54 p.m., Barna Zsombor Klara wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52684/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2016, 5:54 p.m.)
> 
> 
> Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-14754: Track the queries execution lifecycle times
> Five new metrics are being added to track the query execution in HiveServer2. 
> - hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
> - hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 1/5/15 minutes 
> - hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 1/5/15 minutes
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
>   common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
>   service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52684/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
> Unit tests added for the new MetricsQueryLifeTimeHook class.
> Manually tested the new metrics by executing queries in HS2.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Barna Zsombor Klara <zs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52684/
-----------------------------------------------------------

(Updated Feb. 3, 2017, 10:02 a.m.)


Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.


Changes
-------

Added screenshots of the new metrics as shown in the HS2 webUI.


Bugs: HIVE-14754
    https://issues.apache.org/jira/browse/HIVE-14754


Repository: hive-git


Description
-------

HIVE-14754: Track the queries execution lifecycle times
Five new metrics are being added to track the query execution in HiveServer2. 
- hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
- hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
- hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
- hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 1/5/15 minutes 
- hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 1/5/15 minutes


Diffs
-----

  common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
  common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
  common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
  service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
  service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 

Diff: https://reviews.apache.org/r/52684/diff/


Testing
-------

Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
Unit tests added for the new MetricsQueryLifeTimeHook class.
Manually tested the new metrics by executing queries in HS2.


File Attachments (updated)
----------------

New metrics showing query lifecycle timings
  https://reviews.apache.org/media/uploaded/files/2017/02/03/ea41fd61-d49a-49cd-b31f-4567186cfc16__Query_Lifecycle_Timing_Metrics.png
New metrics showing query success rate
  https://reviews.apache.org/media/uploaded/files/2017/02/03/1d613941-5d49-438e-bac9-0b08db190e6b__Query_Success_Rate_Matrics.png


Thanks,

Barna Zsombor Klara


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52684/#review152673
-----------------------------------------------------------


Ship it!




Ship It!

- Peter Vary


On Oct. 10, 2016, 5:54 p.m., Barna Zsombor Klara wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52684/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2016, 5:54 p.m.)
> 
> 
> Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-14754: Track the queries execution lifecycle times
> Five new metrics are being added to track the query execution in HiveServer2. 
> - hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
> - hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 1/5/15 minutes 
> - hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 1/5/15 minutes
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
>   common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
>   service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52684/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
> Unit tests added for the new MetricsQueryLifeTimeHook class.
> Manually tested the new metrics by executing queries in HS2.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Barna Zsombor Klara <zs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52684/
-----------------------------------------------------------

(Updated Oct. 10, 2016, 5:54 p.m.)


Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.


Changes
-------

-Moved constants from Metrics to MetricsConstant
-Updated the unit test for the executing/compiling query count tests
-Used mock SessionState instead of a real instance in the unit tests
-Addressed/answered the nits


Repository: hive-git


Description (updated)
-------

HIVE-14754: Track the queries execution lifecycle times
Five new metrics are being added to track the query execution in HiveServer2. 
- hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
- hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
- hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
- hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 1/5/15 minutes 
- hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 1/5/15 minutes


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
  common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
  common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
  common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
  common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
  common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
  ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
  service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
  service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 

Diff: https://reviews.apache.org/r/52684/diff/


Testing
-------

Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
Unit tests added for the new MetricsQueryLifeTimeHook class.
Manually tested the new metrics by executing queries in HS2.


Thanks,

Barna Zsombor Klara


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Peter Vary <pv...@cloudera.com>.

> On Oct. 10, 2016, 1:37 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 581
> > <https://reviews.apache.org/r/52684/diff/1/?file=1529597#file1529597line581>
> >
> >     Why this MetricsQueryLifeTimeHook is not another one of the HIVE_QUERY_LIFETIME_HOOKS?
> 
> Barna Zsombor Klara wrote:
>     There are no other implementations of this interface currently in Hive. It can/should be user supplied. However I thought that enabling the metric gathering should not be driven by more than one property. If I add it as just another query lifetime hook, then it would be the user's responsability to add it to the query lifetime hook property list. This seems a bit confusing to me, but I'm open for other suggestions.

Your reasoning is sound, and with that I think we should stick to your original version.


> On Oct. 10, 2016, 1:37 p.m., Peter Vary wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java, line 78
> > <https://reviews.apache.org/r/52684/diff/1/?file=1529599#file1529599line78>
> >
> >     Maybe this is just me not understanding the description for the metrics...
> >     You wrote this in the description:
> >     "- hs2_executing_queries - is showing the number of queries currently in the execution phase "
> >     
> >     After calling the beforeExecution method I think the count should be 1, since we have 1 query in the execution phase
> >     
> >     The same for HS2_COMPILING_QUERIES, if my comment above is valid.
> 
> Barna Zsombor Klara wrote:
>     The timer will only increase its counter once the measurement is done, so once the query has finished compiling or executing. We have a counter to track the number of queries being executed/compiled with the same MetricsConstant value. I have updated the test to make this clearer.

Thanks for the info.


- Peter


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


On Oct. 10, 2016, 5:54 p.m., Barna Zsombor Klara wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52684/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2016, 5:54 p.m.)
> 
> 
> Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-14754: Track the queries execution lifecycle times
> Five new metrics are being added to track the query execution in HiveServer2. 
> - hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
> - hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 1/5/15 minutes 
> - hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 1/5/15 minutes
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
>   common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
>   service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52684/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
> Unit tests added for the new MetricsQueryLifeTimeHook class.
> Manually tested the new metrics by executing queries in HS2.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Barna Zsombor Klara <zs...@cloudera.com>.

> On Oct. 10, 2016, 1:37 p.m., Peter Vary wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 581
> > <https://reviews.apache.org/r/52684/diff/1/?file=1529597#file1529597line581>
> >
> >     Why this MetricsQueryLifeTimeHook is not another one of the HIVE_QUERY_LIFETIME_HOOKS?

There are no other implementations of this interface currently in Hive. It can/should be user supplied. However I thought that enabling the metric gathering should not be driven by more than one property. If I add it as just another query lifetime hook, then it would be the user's responsability to add it to the query lifetime hook property list. This seems a bit confusing to me, but I'm open for other suggestions.


> On Oct. 10, 2016, 1:37 p.m., Peter Vary wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java, line 78
> > <https://reviews.apache.org/r/52684/diff/1/?file=1529599#file1529599line78>
> >
> >     Maybe this is just me not understanding the description for the metrics...
> >     You wrote this in the description:
> >     "- hs2_executing_queries - is showing the number of queries currently in the execution phase "
> >     
> >     After calling the beforeExecution method I think the count should be 1, since we have 1 query in the execution phase
> >     
> >     The same for HS2_COMPILING_QUERIES, if my comment above is valid.

The timer will only increase its counter once the measurement is done, so once the query has finished compiling or executing. We have a counter to track the number of queries being executed/compiled with the same MetricsConstant value. I have updated the test to make this clearer.


> On Oct. 10, 2016, 1:37 p.m., Peter Vary wrote:
> > service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java, line 52
> > <https://reviews.apache.org/r/52684/diff/1/?file=1529602#file1529602line52>
> >
> >     I am not sure that this is important but creating a new SessionState runs a quite big chunk of code. Is it possible to use only one SessionState in the test, and close() it at tearDown?

I can change it to a mock.


- Barna Zsombor


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


On Oct. 10, 2016, 5:54 p.m., Barna Zsombor Klara wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52684/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2016, 5:54 p.m.)
> 
> 
> Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-14754: Track the queries execution lifecycle times
> Five new metrics are being added to track the query execution in HiveServer2. 
> - hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
> - hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 1/5/15 minutes 
> - hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 1/5/15 minutes
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
>   common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
>   service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52684/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
> Unit tests added for the new MetricsQueryLifeTimeHook class.
> Manually tested the new metrics by executing queries in HS2.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>


Re: Review Request 52684: HIVE-14754: Track the queries execution lifecycle times

Posted by Peter Vary <pv...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52684/#review151968
-----------------------------------------------------------



Hi Zsombor,

Thanks for the great patch.
Minor nits, and some questions - most probably some stuff is not clear for me :)

Thanks,
Peter


common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java (line 27)
<https://reviews.apache.org/r/52684/#comment220647>

    If we refactor this, isn't this better placed in MetricsConstant?



common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java (line 104)
<https://reviews.apache.org/r/52684/#comment220648>

    In your patch description it is 5/10/15 min :) Reading the Meter javadoc, 1/5/15 seems correct to me



common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java (line 71)
<https://reviews.apache.org/r/52684/#comment220645>

    Might be a good idea to have a comment here for the new metrics



ql/src/java/org/apache/hadoop/hive/ql/Driver.java (line 579)
<https://reviews.apache.org/r/52684/#comment220639>

    Why this MetricsQueryLifeTimeHook is not another one of the HIVE_QUERY_LIFETIME_HOOKS?



ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java (line 78)
<https://reviews.apache.org/r/52684/#comment220640>

    Maybe this is just me not understanding the description for the metrics...
    You wrote this in the description:
    "- hs2_executing_queries - is showing the number of queries currently in the execution phase "
    
    After calling the beforeExecution method I think the count should be 1, since we have 1 query in the execution phase
    
    The same for HS2_COMPILING_QUERIES, if my comment above is valid.



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java (line 667)
<https://reviews.apache.org/r/52684/#comment220641>

    nit: space after if



service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java (line 670)
<https://reviews.apache.org/r/52684/#comment220642>

    nit: space after if



service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java (line 52)
<https://reviews.apache.org/r/52684/#comment220644>

    I am not sure that this is important but creating a new SessionState runs a quite big chunk of code. Is it possible to use only one SessionState in the test, and close() it at tearDown?


- Peter Vary


On Oct. 10, 2016, 12:16 p.m., Barna Zsombor Klara wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52684/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2016, 12:16 p.m.)
> 
> 
> Review request for hive, Gabor Szadovszky, Marta Kuczora, Mohit Sabharwal, Peter Vary, Sergio Pena, and Szehon Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-14754: Track the queries execution lifecycle times
> Five new metrics are being added to track the query execution in HiveServer2. 
> - hs2_submitted_queries - is showing the number of queries currently submitted to HS2 and being processed as well as the min/max/mean execution times for the last 1028 measurements. 
> - hs2_compiling_queries - is showing the number of queries currently in the compilation phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_executing_queries - is showing the number of queries currently in the execution phase and the min/max/mean execution times for the last 1028 measurements. 
> - hs2_failed_queries - is showing the total number of failed queries, as well as the query failure rate exponentially weighted moving average for the last 5/10/15 minutes 
> - hs2_suceeded_queries - is showing the total number of successful queries, as well as the query success rate exponentially weighted moving average for the last 5/10/15 minutes
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java ba2267b29eaeb497e9582ca3ff3de4ad63bf8482 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java 9b263d91321f141adf73e5421f3d659f80081471 
>   common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java c9d4087c5aeaf7f1ca0e9b5860b898b5766cd911 
>   common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java 9525b452eada8ab6a2dd56895f9c9d4eb50db894 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 7658f1c3e0ae7112d10aaf195197ed7e0d318351 
>   common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java 46676589e6656d0f13f1931bfe67a63dd1920042 
>   common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java 6ee6245c1212c06c2ca9cc7a795f288c3928d675 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 530d2f486de9ee92fc627404a3ae1ad086549e19 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java dd5543445ecb7387fad0cb861b9eb720edd06250 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/MetricsQueryLifeTimeHook.java PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/hooks/TestMetricsQueryLifeTimeHook.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 36c6f938316b65cc5444bb4d425066d26847ed70 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java abdf8cd2f7abe2356e86b0eb19cf13311240deb8 
>   service/src/test/org/apache/hive/service/cli/operation/TestSQLOperationMetrics.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52684/diff/
> 
> 
> Testing
> -------
> 
> Unit tests added for existing and new metrics in the SQLOperation and CodahaleMetrics classes.
> Unit tests added for the new MetricsQueryLifeTimeHook class.
> Manually tested the new metrics by executing queries in HS2.
> 
> 
> Thanks,
> 
> Barna Zsombor Klara
> 
>