----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35181/#review86952 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java <https://reviews.apache.org/r/35181/#comment139167> How so? I think implementations do need to be thread safe. For example, how does MetricsFactory help with a thread safe implementation of incCounter? common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java <https://reviews.apache.org/r/35181/#comment139164> comment why this is volatile. common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java <https://reviews.apache.org/r/35181/#comment139165> might want to consider renaming this to something shorter like "get()" or "getInstance()". Also mention that this returns null if init() hasn't been called. common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java <https://reviews.apache.org/r/35181/#comment139166> while you are here, consider renaming deInit() -> shutdown() or close()? - Lenni Kuff On June 6, 2015, 10:18 p.m., Szehon Ho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35181/ > ----------------------------------------------------------- > > (Updated June 6, 2015, 10:18 p.m.) > > > Review request for hive and Sergey Shelukhin. > > > Repository: hive-git > > > Description > ------- > > Eliminated the redundant conf checks and eliminate synchronization in the > code path, by making the static Metrics instance as a static volatile > variable. Achieved this by removing the Metrics init() method and moved > directly to the constructor. > > Left some of the synchronization in the old LegacyMetrics the same. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java c3949f2 > common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java > 14f7afb > common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java > 13a5336 > > common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java > 12a309d > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java > e59da99 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 85a734c > service/src/java/org/apache/hive/service/server/HiveServer2.java 7820ed5 > > Diff: https://reviews.apache.org/r/35181/diff/ > > > Testing > ------- > > Ran affected tests, ran HS2 with and without metrics enabled. > > > Thanks, > > Szehon Ho > >
