[ 
https://issues.apache.org/jira/browse/HADOOP-10660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14020024#comment-14020024
 ] 

Ted Yu commented on HADOOP-10660:
---------------------------------

{code}
+            } else {
+              throw new Exception("Writer in GraphiteSink is null!");
+            }
         } catch (Exception e) {
             throw new MetricsException("Error sending metrics", e);
{code}
The Exception would be caught and converted to MetricsException - better throw 
MetricsException in the first place.
{code}
+        if(writer != null){
+          writer.close();
+          LOG.info("GraphiteSink "+this.toString()+" is closed!");
{code}
writer should be set to null after close() returns.
nit: .toString() is not needed.

Should socket become a member of GraphiteSink and be closed in close() ?

> GraphiteSink should implement Closeable
> ---------------------------------------
>
>                 Key: HADOOP-10660
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10660
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: Chen He
>         Attachments: HADOOP-10660-v2.patch, HADOOP-10660.patch
>
>
> GraphiteSink wraps OutputStreamWriter around socket's output stream.
> Currently the socket is never closed.
> GraphiteSink should implement Closeable such that MetricsSystem can close the 
> socket when it is stopped.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to