HADOOP-18129: Change URI to String in INodeLink to reduce memory footprint of ViewFileSystem #3996
Conversation
This file contains 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. |
ibuenros
suggested changes
Feb 18, 2022
| } | ||
|
|
||
| public Path getMountedOnPath() { | ||
| return mountedOnPath; | ||
| } | ||
|
|
||
| public URI[] getTargetFileSystemURIs() { | ||
| return targetFileSystemURIs; | ||
| public String[] getTargetFileSystemPaths() { |
There was a problem hiding this comment.
This is a public method, can we keep the old signature? We can transform from string to URI within the method itself.
There was a problem hiding this comment.
Added getTargetFileSystemURIs where we are returning URI[]. Also added method to return targetFileSystem Paths.
| targetDirLinkList = new URI[1]; | ||
| targetDirLinkList[0] = aTargetDirLink; | ||
| targetDirLinkList = new String[1]; | ||
| targetDirLinkList[0] = new URI(aTargetDirLink).toString(); |
There was a problem hiding this comment.
What is the purpose of creating a URI and converting back to string?
There was a problem hiding this comment.
In the previous implementation, we were creating the URI object from the target file system path. If the path is not a valid URI, the filesystem initialization should have failed. As we are not keeping the URI object anymore, this ensures we are not dealing with an invalid URI and new URI object validates the target filesystem path.
abhishekdas99
force-pushed
the
HADOOP-18129
branch
from
a75520f
to
d69c720
Compare
The performance impact of this change where we are creating the URI object when its needed as well as creating a dummy URI object to check the validity of the path. |
|
🎊 +1 overall
This message was automatically generated. |
|
What do the numbers look like for the memory usage before and after this change? |
For 40k mount points, the size of ViewFileSystem reduced from 62MB to 28MB. |
omalley
pushed a commit
that referenced
this pull request
Mar 18, 2022
…print of ViewFileSystem Fixes #3996
abhishekdas99
added a commit
to abhishekdas99/hadoop
that referenced
this pull request
Mar 18, 2022
…print of ViewFileSystem Fixes apache#3996 (cherry picked from commit da9970d)
Closed
4 tasks
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…print of ViewFileSystem Fixes apache#3996
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
https://issues.apache.org/jira/browse/HADOOP-18129
Description of PR
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?