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

Chris Nauroth commented on MAPREDUCE-5508:
------------------------------------------

Thanks for the new patch, Xi.  This mostly looks good to me, and I'm glad to 
hear that it still fixes the memory leak.  Here are a few comments:

# Can we remove the unused {{PathDeletionContext}} constructor?  It would 
require a small change in {{TestCleanupQueue}}.
# Swallowing the {{InterruptedException}} is problematic if any upstream code 
depends on seeing the thread's interrupted status, so let's restore the 
interrupted status in the catch block by calling 
{{Thread.currentThread().interrupt()}}.
# If there is an {{InterruptedException}}, then we currently would pass a null 
{{tempDirFs}} to the {{CleanupQueue}}, where we'd once again risk leaking 
memory.  I suggest that if there is an {{InterruptedException}}, then we skip 
adding to the {{CleanupQueue}} and log a warning.  This is consistent with the 
error-handling strategy in the rest of the method.  (It logs warnings.)

                
> JobTracker memory leak caused by unreleased FileSystem objects in 
> JobInProgress#cleanupJob
> ------------------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-5508
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-5508
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: jobtracker
>    Affects Versions: 1-win, 1.2.1
>            Reporter: Xi Fang
>            Assignee: Xi Fang
>            Priority: Critical
>         Attachments: MAPREDUCE-5508.1.patch, MAPREDUCE-5508.patch
>
>
> MAPREDUCE-5351 fixed a memory leak problem but introducing another filesystem 
> object (see "tempDirFs") that is not properly released.
> {code} JobInProgress#cleanupJob()
>   void cleanupJob() {
> ...
>           tempDirFs = jobTempDirPath.getFileSystem(conf);
>           CleanupQueue.getInstance().addToQueue(
>               new PathDeletionContext(jobTempDirPath, conf, userUGI, jobId));
> ...
>  if (tempDirFs != fs) {
>       try {
>         fs.close();
>       } catch (IOException ie) {
> ...
> }
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to