> On Oct. 17, 2017, 11:21 p.m., Vihang Karajgaonkar wrote: > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java > > Lines 119 (patched) > > <https://reviews.apache.org/r/62995/diff/2/?file=1858825#file1858825line119> > > > > I think it is better to throw a RuntimeException here instead of > > catching it since there is nothing more that you can do if the metricsDir > > could not be created and it doesn't exist. > > Alexander Kolbasov wrote: > Here we actually avoid creating JSON reporter. My thinking is that it > isn't a catastrophic faiilure so you should be able to continue running > without it. Do you think that it is better to throw an exception rather then > continue running without the reporter? > > Vihang Karajgaonkar wrote: > If we catch the exception and return then the ExecutorService remains > uninitialized and when CodahaleMetrics class tries to close it sometime later > it will result in a NPE on ExecutorService.shutdown(). I think throwing this > exception and handling it at CodahaleMetrics such that this reporter is not > added in list of reporters would be cleaner way to handle this error.
Added a check in close() to avoid NPE. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62995/#review188416 ----------------------------------------------------------- On Oct. 17, 2017, 12:35 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62995/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2017, 12:35 a.m.) > > > Review request for hive, Aihua Xu, Andrew Sherman, Janaki Lahorani, Sergio > Pena, Sahil Takiar, and Vihang Karajgaonkar. > > > Bugs: HIVE-17806 > https://issues.apache.org/jira/browse/HIVE-17806 > > > Repository: hive-git > > > Description > ------- > > HIVE-17806 Create directory for metrics file if it doesn't exist > > > Diffs > ----- > > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/JsonFileMetricsReporter.java > 96243cb74a154b9a639ffb080256c4b43bd35a4b > > common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java > 254af7d4310578e3883c0dffa64bed0f823696ea > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/metrics/JsonReporter.java > f71bb25463b56bc741f989454664397996b6a5cf > > > Diff: https://reviews.apache.org/r/62995/diff/2/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
