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

Jason Lowe commented on MAPREDUCE-6621:
---------------------------------------

Patch looks mostly good, just a nit that the comment is misleading.  Setting 
'prev' to null doesn't accomplish much -- it's just a local variable that's 
just about to go out of scope anyway.  The comment implies it's important that 
it is set to null, but it has nothing to do with the issue.  The null setting 
from MAPREDUCE-6618 was important since those were instance variables that 
survive the current method.

We can skip the unnecessary null, and the comment should just say it's 
important to close the previous cluster instance to cleanup resources.  IMHO 
the debug log is overkill, but I'm OK if you want to leave it.


> Memory Leak in JobClient#submitJobInternal()
> --------------------------------------------
>
>                 Key: MAPREDUCE-6621
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-6621
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>            Reporter: Xuan Gong
>            Assignee: Xuan Gong
>         Attachments: MAPREDUCE-6621.1.patch
>
>
> In JobClient:
> {code}
> public RunningJob submitJobInternal(final JobConf conf)
>       throws FileNotFoundException, IOException {
>     try {
>       conf.setBooleanIfUnset("mapred.mapper.new-api", false);
>       conf.setBooleanIfUnset("mapred.reducer.new-api", false);
>       Job job = clientUgi.doAs(new PrivilegedExceptionAction<Job> () {
>         @Override
>         public Job run() throws IOException, ClassNotFoundException, 
>           InterruptedException {
>           Job job = Job.getInstance(conf);
>           job.submit();
>           return job;
>         }
>       });
>       // update our Cluster instance with the one created by Job for 
> submission
>       // (we can't pass our Cluster instance to Job, since Job wraps the 
> config
>       // instance, and the two configs would then diverge)
>       cluster = job.getCluster();
>       return new NetworkedJob(job);
>     } catch (InterruptedException ie) {
>       throw new IOException("interrupted", ie);
>     }
>   }
> {code}
> We will replace the cluster object with the cluster object from Job, but the 
> previous old cluster object would never be closed.



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

Reply via email to