HDFS-16031. Possible Resource Leak in org.apache.hadoop.hdfs.server.aliasmap#InMemoryAliasMap #3027
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
aajisaka
requested changes
May 20, 2021
Member
aajisaka
left a comment
aajisaka
left a comment
There was a problem hiding this comment.
Thanks for the patch.
We should use try-with-resources to close the resources.
Contributor
Author
|
💔 -1 overall
This message was automatically generated. |
aajisaka
requested changes
May 24, 2021
Comment on lines
329
to
332
| addFileToTarGzRecursively(tOut, aliasMapDir, "", new Configuration()); | ||
| } finally { | ||
| if (tOut != null) { | ||
| tOut.finish(); | ||
| } |
Member
There was a problem hiding this comment.
Thanks for the update.
- Before:
tOut.finish()is called if addFileToTarGzRecursively throws an exception. - Your patch:
tOut.finish()is not called if addFileToTarGzRecursively throws an exception.
I think we need to have an extra try-catch clause:
try {
addFileToTarGzRecursively(tOut, aliasMapDir, "", new Configuration());
} finally {
tOut.finish();
}
tOut cannot be null in the try-with-resources clause so that we can remove the null check.
Contributor
Author
There was a problem hiding this comment.
If we define tOut in try-with-resources block, then we don't have access to tOut in finally block. I checked close() method in TarArchiveOutputStream and finish() is called there so I think we don't need to call finish on tOut.
Contributor
Author
There was a problem hiding this comment.
6a74dd9
let me know if there is any problem.
|
💔 -1 overall
This message was automatically generated. |
Member
|
Thank you @Nargeshdb |
kiran-maturi
pushed a commit
to kiran-maturi/hadoop
that referenced
this pull request
Nov 24, 2021
…liasmap#InMemoryAliasMap (apache#3027)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the issue mentioned here.