----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36241/#review90760 -----------------------------------------------------------
Looks good to me. Just couple nits. common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java (line 188) <https://reviews.apache.org/r/36241/#comment143887> Nit. variable name: decrement? common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java (line 81) <https://reviews.apache.org/r/36241/#comment143888> Same here, and javadoc. decrement? metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 6075) <https://reviews.apache.org/r/36241/#comment143889> Should we use metrics instead of calling MetricsFactory.getInstance() again? Same below. metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 270) <https://reviews.apache.org/r/36241/#comment143892> Do we need to make openTransactionCalls volatile? This variable is accessed from multiple threads, right? - Jimmy Xiang On July 7, 2015, 3 a.m., Szehon Ho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36241/ > ----------------------------------------------------------- > > (Updated July 7, 2015, 3 a.m.) > > > Review request for hive. > > > Bugs: HIVE-10927 > https://issues.apache.org/jira/browse/HIVE-10927 > > > Repository: hive-git > > > Description > ------- > > Adds following new metrics to HMS, can be renamed if necessary. > > "open_connections"; //#HMS clients connecting to HMS > > "active_jdo_transactions"; //#active JDO transactions > "rollbacked_jdo_transactions"; //#failed JDO transactions > "committed_jdo_transactions"; //#successful JDO transactions > "opened_jdo_transactions"; //#attempted JDO transactions > > Also to HS2: > "open_connections"; //#HMS clients connecting to HMS > > > This also fixes some minor issues: > 1. For the metrics JSON-file-reporter, the file system was not chosen right. > Fixing that, and also making the default the local file system. > 2. The metrics was getting closed in the metastore in the wrong place, > fixing it. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java ec5ac4a > common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java > e811339 > common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java > 27b69cc > > common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java > PRE-CREATION > > common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsVariable.java > PRE-CREATION > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java > ae353d0 > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6d0cf15 > > common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java > 954b388 > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMetrics.java > 25f34d1 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 0bcd053 > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 4273c0b > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > dfb7faa > > Diff: https://reviews.apache.org/r/36241/diff/ > > > Testing > ------- > > Adding some unit tests, and tested manually for HS2 which cannot be > unit-tested. > > > Thanks, > > Szehon Ho > >
