hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vihang Karajgaonkar via Review Board <nore...@reviews.apache.org>
Subject Re: Review Request 62995: HIVE-17806 Create directory for metrics file if it doesn't exist
Date Wed, 18 Oct 2017 02:52:41 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message