> 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?
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. - Vihang ----------------------------------------------------------- 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 > >
