-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix potential fd leakage for SegmentProcessorFramework #9797
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9797 +/- ##
=============================================
- Coverage 64.08% 25.27% -38.82%
+ Complexity 4970 44 -4926
=============================================
Files 1903 1944 +41
Lines 102553 104622 +2069
Branches 15605 15854 +249
=============================================
- Hits 65722 26441 -39281
- Misses 32062 75472 +43410
+ Partials 4769 2709 -2060
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
outputSegmentDirs.add(driver.getOutputDirectory()); | ||
} | ||
} finally { | ||
fileManager.cleanUp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like we can assert _mapperOutputDir is cleaned in SegmentProcessorFramework
before calling FileUtils.cleanDirectory(workingDir);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_mapperOutputDir and _reducerOutputDir are internal to this framework, so the caller can't check their existence. And when the issue happened, the files were deleted (file names were gone from the file system). But as it's still opened by the process via mmap, so the file (inode, disk space) was not released cleanly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. one comment on adding tests
Try to fix #9415: potential fd leakage in SegmentProcessorFramework. The fileManager has to call cleanup to release the mmap'ed intermediate files.