Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============5969382087209633599=="
MIME-Version: 1.0
Subject: Re: Review Request 52684: HIVE-14754: Track the queries execution
 lifecycle times
From: Peter Vary <pvary@cloudera.com>
To: Szehon Ho <szehon@cloudera.com>, Mohit Sabharwal <mohit@cloudera.com>,
 Marta Kuczora <kuczoram@cloudera.com>, Peter Vary <pvary@cloudera.com>,
 Sergio Pena <sergio.pena@cloudera.com>,
 Gabor Szadovszky <gabor.szadovszky@cloudera.com>
Cc: Barna Zsombor Klara <zsombor.klara@cloudera.com>,
 hive <dev@hive.apache.org>
Date: Fri, 14 Oct 2016 08:15:43 -0000
Message-ID: <20161014081543.19442.76489@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: Peter Vary <noreply@reviews.apache.org>
References: <20161010133751.23091.79617@reviews.apache.org>
In-Reply-To: <20161010133751.23091.79617@reviews.apache.org>
Reply-To: Peter Vary <pvary@cloudera.com>
archived-at: Fri, 14 Oct 2016 08:15:45 -0000

--===============5969382087209633599==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


--===============5969382087209633599==--