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