> On Sept. 25, 2017, 7:05 p.m., Sahil Takiar wrote: > > Changes look good. Just to confirm, the changes to the unit tests include > > an explicit test for the bug reported in HIVE-17563, correct? IOW, there is > > a test that first writes the metrics files, updates the metrics in HS2, > > re-writes the metrics file, and then validates that the re-written metrics > > file contains the updated values?
The tests for both classes do include the test that verifies that the JSON file is updated every time the metric is updated along the lines that you described. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62487/#review186149 ----------------------------------------------------------- On Sept. 22, 2017, 11:46 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, 11:46 p.m.) > > > Review request for hive, Aihua Xu, Carl Steinbach, Alan Gates, 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/5/ > > > Testing > ------- > > TestCodahaleMetrics tests that the json reporter functionality > > > Thanks, > > Alexander Kolbasov > >
