You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Kevin Wilfong <ke...@fb.com> on 2011/10/07 21:42:21 UTC
Review Request: Add reset operation and average time attribute to Metrics
MBean.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2290/
-----------------------------------------------------------
Review request for hive and Paul Yang.
Summary
-------
I added a reset operation which sets the value of all attributes to 0, see https://issues.apache.org/jira/browse/HIVE-2490 for an explanation of why 0 was chosen and why the attributes are not deleted.
I also added an average time attribute in addition to number and total time. This seemed natural as we have all the data we need to calculate it.
Previously, because we were incrementing the number at the beginning of the function, and the total time at the end of the function, it was impossible to guarantee an accurate average. In particular if the frequency of function calls was high and/or the execution time of the function was very high, the average that could be determined based on the given attributes could be very inaccurate.
I also added some synchronizations to the Metrics class where I thought it did not appear to be thread safe.
I based the average and reset API on what is done by Zookeeper in Hadoop.
This addresses bug HIVE-2490.
https://issues.apache.org/jira/browse/HIVE-2490
Diffs
-----
trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 1179884
trunk/common/src/java/org/apache/hadoop/hive/common/metrics/MetricsMBeanImpl.java 1179884
Diff: https://reviews.apache.org/r/2290/diff
Testing
-------
I ran an instance of the Metastore, and executed queries against it from multiple clients, and verified the average attribute and reset operation behaved as expected.
Thanks,
Kevin
Re: Review Request: Add reset operation and average time attribute to
Metrics MBean.
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2290/#review5298
-----------------------------------------------------------
Ship it!
+1 Looks good. Can you upload the patch on jira so that I can test and commit.
- Ashutosh
On 2011-10-07 19:42:21, Kevin Wilfong wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2290/
> -----------------------------------------------------------
>
> (Updated 2011-10-07 19:42:21)
>
>
> Review request for hive and Paul Yang.
>
>
> Summary
> -------
>
> I added a reset operation which sets the value of all attributes to 0, see https://issues.apache.org/jira/browse/HIVE-2490 for an explanation of why 0 was chosen and why the attributes are not deleted.
>
> I also added an average time attribute in addition to number and total time. This seemed natural as we have all the data we need to calculate it.
>
> Previously, because we were incrementing the number at the beginning of the function, and the total time at the end of the function, it was impossible to guarantee an accurate average. In particular if the frequency of function calls was high and/or the execution time of the function was very high, the average that could be determined based on the given attributes could be very inaccurate.
>
> I also added some synchronizations to the Metrics class where I thought it did not appear to be thread safe.
>
> I based the average and reset API on what is done by Zookeeper in Hadoop.
>
>
> This addresses bug HIVE-2490.
> https://issues.apache.org/jira/browse/HIVE-2490
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java 1179884
> trunk/common/src/java/org/apache/hadoop/hive/common/metrics/MetricsMBeanImpl.java 1179884
>
> Diff: https://reviews.apache.org/r/2290/diff
>
>
> Testing
> -------
>
> I ran an instance of the Metastore, and executed queries against it from multiple clients, and verified the average attribute and reset operation behaved as expected.
>
>
> Thanks,
>
> Kevin
>
>