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

Jason Lowe commented on MAPREDUCE-6618:
---------------------------------------

Thanks for updating the patch!  Couple of nits:

Should we be setting fields to null as we check them for non-null and close 
them?

Is there a reason we use entrySet() instead of values() here?  It ignores the 
job IDs, so thinking we don't need the full entry set.
{code}
      for (Entry<JobID, ClientServiceDelegate> delegate : cache.entrySet()) {
        ClientServiceDelegate jobDelegate = delegate.getValue();
        if (jobDelegate != null) {
          jobDelegate.close();
        }
      }
{code}

> YarnClientProtocolProvider leaking the YarnClient thread. 
> ----------------------------------------------------------
>
>                 Key: MAPREDUCE-6618
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-6618
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: MAPREDUCE-6618.1.patch, MAPREDUCE-6618.2.patch, 
> MAPREDUCE-6618.3.patch
>
>
> YarnClientProtocolProvider creates YarnRunner which includes 
> ResourceMgrDelegate. In ResourceMgrDelegate, we would initiate and start 
> yarnclient. The yarnClient thread would be leaked due to
> {code}
>   @Override
>   public void close(ClientProtocol clientProtocol) throws IOException {
>     // nothing to do
>   }
> {code} in YarnClientProtocolProvider



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to