----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57632/#review169775 -----------------------------------------------------------
Thanks for the changes Sunitha. Few comments below. common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java Line 392 (original), 381 (patched) <https://reviews.apache.org/r/57632/#comment242372> Does code does not fallback to initMetricsReported() if initCodahaleMetricsReporterClasses() throws an exception? common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleReporter.java Lines 22 (patched) <https://reviews.apache.org/r/57632/#comment242371> I think this can be renamed to something more generic like MetricsReporter. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 2207 (patched) <https://reviews.apache.org/r/57632/#comment242368> Nit, spell check -> separated instead of seperated. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 2213 (patched) <https://reviews.apache.org/r/57632/#comment242369> If you intent to deprecate this config use @deprecated instead. Also, instead of "- will be .. " it would be good if it says "This configuration will be ..." common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java Line 62 (original), 63 (patched) <https://reviews.apache.org/r/57632/#comment242375> Can you add additional testcases to verify that older config gets used when the newer one is not set? - Vihang Karajgaonkar On March 21, 2017, 4:05 p.m., Sunitha Beeram wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57632/ > ----------------------------------------------------------- > > (Updated March 21, 2017, 4:05 p.m.) > > > Review request for hive, Carl Steinbach and Ratandeep Ratti. > > > Bugs: Hive-16206 > https://issues.apache.org/jira/browse/Hive-16206 > > > Repository: hive-git > > > Description > ------- > > HIVE-16206: Address review comments > > > Diffs > ----- > > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java > e8abf6cf06afc9fa590af3a447eacc67735a69e6 > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleReporter.java > PRE-CREATION > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/ConsoleMetricsReporter.java > PRE-CREATION > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JmxMetricsReporter.java > PRE-CREATION > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java > PRE-CREATION > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/Metrics2Reporter.java > PRE-CREATION > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 1fb32533d58af4ec622feb320bf9315da5db6e76 > > common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java > aa4e75f9f8160d1b54b14c1a23ea42e156bd45ca > > > Diff: https://reviews.apache.org/r/57632/diff/4/ > > > Testing > ------- > > Updated unit tests and all unit tests passed locally. > > > Thanks, > > Sunitha Beeram > >
