----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62487/#review186011 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java Line 75 (original), 116 (patched) <https://reviews.apache.org/r/62487/#comment262364> I think the old implementation called `shutdown()` instead of `shutdownNow()`. It may be best to call `shutdown()` first, call `awaitTermination(30s)`, and then call `shutdownNow()`. common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java Line 77 (original), 118 (patched) <https://reviews.apache.org/r/62487/#comment262365> I don't know if we want this file to be deleted. The `close()` method is basically invoked when HS2 is shutdown, but there may be external systems who read the metrics after HS2 has been shutdown. common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java Lines 157 (patched) <https://reviews.apache.org/r/62487/#comment262366> This looks like it should go into a `finally` block. common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java Line 118 (original), 166 (patched) <https://reviews.apache.org/r/62487/#comment262367> This looks like it should go into a `finally` block. standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java Lines 107 (patched) <https://reviews.apache.org/r/62487/#comment262368> Might want to change this to something more informative, like `logging metrics with frequency: ` - Sahil Takiar On Sept. 22, 2017, 6:14 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62487/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2017, 6:14 p.m.) > > > Review request for hive, Aihua Xu, Sergio Pena, Sahil Takiar, and Vihang > Karajgaonkar. > > > Bugs: HIVE-17563 > https://issues.apache.org/jira/browse/HIVE-17563 > > > Repository: hive-git > > > Description > ------- > > HIVE-17563: CodahaleMetrics.JsonFileReporter is not updating > hive.service.metrics.file.location > > > Diffs > ----- > > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java > c07517a634e35c936d6ea68e9a2874ac2c59929a > > common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java > 67f81d6c43a7904dc384cfcce6f948f1f802144c > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java > b804cdade07079955a65cb431fab078dcecd53b1 > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/metrics/TestMetrics.java > 259a4db43905c47d873776df96210a4e77d07076 > > > Diff: https://reviews.apache.org/r/62487/diff/4/ > > > Testing > ------- > > TestCodahaleMetrics tests that the json reporter functionality > > > Thanks, > > Alexander Kolbasov > >
