[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767702#comment-13767702 ] Xi Fang commented on MAPREDUCE-5508: Thanks [~sandyr] and [~cnauroth]. Actually, the above discussion made me have second thoughts on the patch attached. There is a race condition here. Supposed that Path#getFileSystem in CleanupQueue#deletePath retrieved the same instance of JobInProgress#fs from FileSystem#Cache as well. Because there is race condition between DistributedFileSystem#close() and FileSystem#close(), it is possible that at the most just after JobInProgress#cleanupJob closed JobInProgress#fs's DFSClient, the processor switched to CleanupQueue#deletePath and called fs.delete(). Because this fs's DFCClient has been closed, an exception would be thrown and this staging directory won't be deleted then. > 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.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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767698#comment-13767698 ] Chris Nauroth commented on MAPREDUCE-5508: -- bq. If I understand correctly, in that case the same UGI instance (JobInProgress.userUGI) that was used to create the fs is used to close it, so having different subjects is not possible. The 2 UGI instances (one passed in to {{PathDeletionContext}} and one created implicitly by the call to {{Path#getFileSystem}}) do have the same value, but they have different identities. Even though they are logically equivalent, there are 2 different underlying {{Subject}} instances with different identity hash codes. Thus, the cache entry used during creation of the {{FileSystem}} will be different from the one used during close, which causes the leak. Another way of saying this is that even though the same {{Principal}} is used, {{Path#getFileSystem}} will create a different {{Subject}}, because {{CleanupQueue}} is running inside a different JAAS {{AccessControlContext}}. The {{Subject}} is a function of not only the {{Principal}} but also the {{AccessControlContext}}. (Again, both the {{AccessControlContext}} and {{Subject}} may be logically equivalent, but we're observing that they are not the same instances, so they have different identity hash codes.) If you really want to avoid the extra close, then the only other possible solution that I can think of would involve passing the {{FileSystem}} instance to use into the {{PathDeletionContext}}. This would make it explicit instead of relying on the implicit cache lookup of {{Path#getFileSystem}}. I haven't completely thought through if that approach would have other side effects though. > 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.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
[jira] [Assigned] (MAPREDUCE-5498) maven Junit dependency should be test only
[ https://issues.apache.org/jira/browse/MAPREDUCE-5498?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Chris Nauroth reassigned MAPREDUCE-5498: Assignee: André Kelpe > maven Junit dependency should be test only > -- > > Key: MAPREDUCE-5498 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-5498 > Project: Hadoop Map/Reduce > Issue Type: Bug > Components: build >Affects Versions: 3.0.0, 2.1.0-beta >Reporter: Steve Loughran >Assignee: André Kelpe >Priority: Minor > Attachments: HADOOP-9935-001.patch > > > The maven dependencies for the YARN artifacts don't restrict to test time, so > it gets picked up by all downstream users. -- 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
[jira] [Reopened] (MAPREDUCE-5498) maven Junit dependency should be test only
[ https://issues.apache.org/jira/browse/MAPREDUCE-5498?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Chris Nauroth reopened MAPREDUCE-5498: -- > maven Junit dependency should be test only > -- > > Key: MAPREDUCE-5498 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-5498 > Project: Hadoop Map/Reduce > Issue Type: Bug > Components: build >Affects Versions: 3.0.0, 2.1.0-beta >Reporter: Steve Loughran >Assignee: André Kelpe >Priority: Minor > Attachments: HADOOP-9935-001.patch > > > The maven dependencies for the YARN artifacts don't restrict to test time, so > it gets picked up by all downstream users. -- 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
[jira] [Resolved] (MAPREDUCE-5498) maven Junit dependency should be test only
[ https://issues.apache.org/jira/browse/MAPREDUCE-5498?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Chris Nauroth resolved MAPREDUCE-5498. -- Resolution: Duplicate > maven Junit dependency should be test only > -- > > Key: MAPREDUCE-5498 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-5498 > Project: Hadoop Map/Reduce > Issue Type: Bug > Components: build >Affects Versions: 3.0.0, 2.1.0-beta >Reporter: Steve Loughran >Assignee: André Kelpe >Priority: Minor > Attachments: HADOOP-9935-001.patch > > > The maven dependencies for the YARN artifacts don't restrict to test time, so > it gets picked up by all downstream users. -- 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
[jira] [Updated] (MAPREDUCE-5498) maven Junit dependency should be test only
[ https://issues.apache.org/jira/browse/MAPREDUCE-5498?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Chris Nauroth updated MAPREDUCE-5498: - Resolution: Fixed Status: Resolved (was: Patch Available) I believe this is fixed by the HADOOP-9935 patch, which set {{test}} in the parent hadoop-project/pom.xml, so I'm resolving this as duplicate. Please feel free to reopen if you think there is any work remaining. > maven Junit dependency should be test only > -- > > Key: MAPREDUCE-5498 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-5498 > Project: Hadoop Map/Reduce > Issue Type: Bug > Components: build >Affects Versions: 3.0.0, 2.1.0-beta >Reporter: Steve Loughran >Priority: Minor > Attachments: HADOOP-9935-001.patch > > > The maven dependencies for the YARN artifacts don't restrict to test time, so > it gets picked up by all downstream users. -- 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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767644#comment-13767644 ] Sandy Ryza commented on MAPREDUCE-5508: --- If I understand correctly, in that case the same UGI instance (JobInProgress.userUGI) that was used to create the fs is used to close it, so having different subjects is not possible. > 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.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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767525#comment-13767525 ] Xi Fang commented on MAPREDUCE-5508: Thanks Sandy for the information on HADOOP-6670. I think we may still need to close fs anyway, because p.getFileSystem(conf) in CleanupQueue#deletePath may not be able to find the FileSystem#Cache entry of JobInProgress#fs because of the different subject problem we discussed above. In this case, nothing will remove JobInProgress#fs from the FileSystem#Cache. > 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.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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767509#comment-13767509 ] Sandy Ryza commented on MAPREDUCE-5508: --- Ahh ok that makes total sense. Agreed that we shouldn't change the equals/hashCode - this behavior was intentional (HADOOP-6670). We still shouldn't need to close fs in every case though, right? We should be able to look at the scheme of the jobSubmitDir and only close it if it doesn't match the scheme of jobTempDir? > 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.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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767402#comment-13767402 ] Xi Fang commented on MAPREDUCE-5508: Just found Chris was also working on this thread :). I agree with Chris. Changing the hash code may have a wide impact on existing code that would be risky. > 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.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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767401#comment-13767401 ] Xi Fang commented on MAPREDUCE-5508: [~sandyr] Thanks for your comments. bq. Have you tested this fix. Yes. We have tested this fix on our test cluster (about 130,000 submission). After the workflow was done, we waited for a couple of minutes (jobs were retiring), then forced GC, and then dumped the memory. We manually checked the FileSystem#Cache. There was no memory leak. bq. For your analysis 1. I agree with "it doesn't appear that tempDirFs and fs are ever even ending up equal because tempDirFs is created with the wrong UGI." 2. I think tempDir would be fine because 1) JobInProgess#cleanupJob won't introduce a file system instance for tempDir and 2) the fs in CleanupQueue@deletePath would be reused (i.e. only one instance would exist in FileSystem#Cache). My initial thought was this part has a memory leak. But a test shows that there is no problem here. 3. The problem is actually {code} tempDirFs = jobTempDirPath.getFileSystem(conf); {code} The problem here is that this guy "MAY" (I will explain later) put a new entry in FileSystem#Cache. Note that this would eventually go into UserGroupInformation#getCurrentUser to get a UGI with a current AccessControlContext. CleanupQueue#deletePath won't close this entry because a different UGI (i.e. "userUGI" created in JobInProgress) is used there. Here is the tricky part which we had a long discussion with [~cnauroth] and [~vinodkv]. The problem here is that although we may only have one current user, the following code "MAY" return different subjects. {code} static UserGroupInformation getCurrentUser() throws IOException { AccessControlContext context = AccessController.getContext(); -->Subject subject = Subject.getSubject(context); -< {code} Because the entry of FileSystem#Cache uses identityHashCode of a subject to construct the key, a file system object created by "jobTempDirPath.getFileSystem(conf)" may not be found later when this code is executed again, although we may have the same principle (i.e. the current user). This eventually leads to an unbounded number of file system instances in FileSystem#Cache. Nothing is going to remove them from the cache. Please let me know if you have any questions. > 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.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
[jira] [Commented] (MAPREDUCE-5508) JobTracker memory leak caused by unreleased FileSystem objects in JobInProgress#cleanupJob
[ https://issues.apache.org/jira/browse/MAPREDUCE-5508?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13767396#comment-13767396 ] Chris Nauroth commented on MAPREDUCE-5508: -- I was +1 for Xi's patch, but I didn't want to commit it too hastily so that the participants on MAPREDUCE-5351 would get a chance to review. Sandy, thank you for responding so quickly. bq. Have you tested this fix? Yes, Xi repeated his test run of ~200,000 jobs with this patch, and the heap dump no longer showed leaked instances of {{DistributedFileSystem}}. This is definitely the source of the leak. bq. The deeper problem to me is that we are creating a new UGI, which can have a new subject, which can create a new entry in the FS cache, every time CleanupQueue#deletePath is called with a null UGI. Just to clarify, the problem we saw is that a new {{UserGroupInformation}}/new {{Subject}} gets created regardless of whether or not a null UGI was passed to {{CleanupQueue#deletePath}}. It's buried behind the last line of this code snippet: {code} protected void deletePath() throws IOException, InterruptedException { final Path p = getPathForCleanup(); (ugi == null ? UserGroupInformation.getLoginUser() : ugi).doAs( new PrivilegedExceptionAction() { public Object run() throws IOException { FileSystem fs = p.getFileSystem(conf); {code} The last line eventually hits {{FileSystem#Cache#get}}, which calls {{UserGroupInformation#getCurrentUser}} while constructing the cache key, but that doesn't always result in the same underlying {{Subject}} instance as the one that initially created the FS cache entry. bq. A better fix would be to avoid this, either by having CleanupQueue hold a UGI of the login user for use in these situations or to avoid the doAs entirely when the given UGI is null. We can tell from the heap dump that the leaked instances are associated with the user UGI and not the login UGI, so I don't think there is a way to use the login UGI to remove those cache entries. I don't see a way to avoid the doAs, because we're seeing the leak in the case of the user UGI, so we'd want the impersonation in place. Side note: the return value of {{UserGroupInformation#getLoginUser}} is cached for the lifetime of the process, so it's always going to have the same {{Subject}} instance. That makes it impossible to have a large, visible leak in the FS cache for entries associated to the login user. The only other potential solution I can think of is explicitly passing the {{FileSystem}} instance to use for the delete and close into {{PathDeletionContext}}. Then, it wouldn't need to infer the FS to use by calling {{Path#getFileSystem}} with its implicit creation of a new {{Subject}}. Do you think something like that would work? BTW, this problem also made me wonder if it's incorrect for UGI to use an identity hash code. I couldn't track down the rationale for that. Presumably it's related to performance. The code of {{Subject#hashCode}} combines data from a lot of internal data structures, and it all happens while holding the monitor. This made me think changing the hash code would be too risky. > 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.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