----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/352/#review166 -----------------------------------------------------------
http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment346> Can you make it throw an exception if close() is called when it's not open? http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment348> 100 char limit? http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment335> Needs a space between the ) and the {. Please apply to the rest of the diff. http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment339> Needs space between a comma and the next argument. Please apply to the rest of the diff. http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment336> Needs spaces between }, else, and {. Please apply to the rest of the diff. http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment340> Needs space between the operator. Please apply to the rest of the diff. http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java <https://reviews.apache.org/r/352/#comment347> With the way that MetricsScope is currently implemented, you have to create a new MetricsScope object for every call. Could you modify MetricsScope to have a open() as well as close() methods? http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java <https://reviews.apache.org/r/352/#comment341> Stack trace would be helpful here - Paul On 2011-01-24 17:41:59, Sushanth Sowmyan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/352/ > ----------------------------------------------------------- > > (Updated 2011-01-24 17:41:59) > > > Review request for hive. > > > Summary > ------- > > HIVE-1818 : jmx instrumentation for HiveMetaStore call frequency and duration > > > This addresses bug HIVE-1818. > https://issues.apache.org/jira/browse/HIVE-1818 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/Metrics.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/MetricsMBean.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/common/metrics/MetricsMBeanImpl.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 1063088 > > Diff: https://reviews.apache.org/r/352/diff > > > Testing > ------- > > > Thanks, > > Sushanth > >