HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs #1385
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1732,17 +1732,36 @@ private void validateOzoneObj(OzoneObj obj) throws OMException { | |
| * @param args Key args | ||
| * @throws OMException if file does not exist | ||
| * if bucket does not exist | ||
| * if volume does not exist | ||
| * @throws IOException if there is error in the db | ||
| * invalid arguments | ||
| */ | ||
| public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { | ||
| Preconditions.checkNotNull(args, "Key args can not be null"); | ||
| return getFileStatus(args, null); | ||
| } | ||
|
|
||
| /** | ||
| * OzoneFS api to get file status for an entry. | ||
| * | ||
| * @param args Key args | ||
| * @param clientAddress a hint to key manager, order the datanode in returned | ||
| * pipeline by distance between client and datanode. | ||
| * @throws OMException if file does not exist | ||
| * if bucket does not exist | ||
| * if volume does not exist | ||
| * @throws IOException if there is error in the db | ||
| * invalid arguments | ||
| */ | ||
| public OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress) | ||
| throws IOException { | ||
| Preconditions.checkNotNull(args, "Key args can not be null"); | ||
| String volumeName = args.getVolumeName(); | ||
| String bucketName = args.getBucketName(); | ||
| String keyName = args.getKeyName(); | ||
|
|
||
| return getOzoneFileStatus(volumeName, bucketName, keyName, | ||
| args.getRefreshPipeline(), args.getSortDatanodes(), clientAddress); | ||
| } | ||
|
|
||
| private OzoneFileStatus getOzoneFileStatus(String volumeName, | ||
|
|
@@ -1783,7 +1802,9 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName, | |
|
|
||
| // if the key is a file then do refresh pipeline info in OM by asking SCM | ||
| if (fileKeyInfo != null) { | ||
| if (refreshPipeline) { | ||
| refreshPipeline(fileKeyInfo); | ||
| } | ||
| if (sortDatanodes) { | ||
| sortDatanodeInPipeline(fileKeyInfo, clientAddress); | ||
| } | ||
|
|
@@ -2011,7 +2032,8 @@ private void listStatusFindKeyInTableCache( | |
| * @return list of file status | ||
| */ | ||
| public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive, | ||
| String startKey, long numEntries, String clientAddress) | ||
| throws IOException { | ||
| Preconditions.checkNotNull(args, "Key args can not be null"); | ||
|
|
||
| List<OzoneFileStatus> fileStatusList = new ArrayList<>(); | ||
|
|
@@ -2144,10 +2166,14 @@ public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive, | |
| metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, | ||
| bucketName); | ||
| } | ||
|
|
||
| for (OzoneFileStatus fileStatus : fileStatusList) { | ||
| if (args.getRefreshPipeline()) { | ||
|
There was a problem hiding this comment. @ChenSammi Was this instance of |
||
| refreshPipeline(fileStatus.getKeyInfo()); | ||
| } | ||
| if (args.getSortDatanodes()) { | ||
| sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress); | ||
| } | ||
| } | ||
| return fileStatusList; | ||
| } | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,8 +31,28 @@ | |
| * Ozone Manager FileSystem interface. | ||
| */ | ||
| public interface OzoneManagerFS extends IOzoneAcl { | ||
|
|
||
| /** | ||
| * Get file status for a file or a directory. | ||
| * | ||
| * @param args the args of the key provided by client. | ||
| * @return file status. | ||
| * @throws IOException if file or bucket or volume does not exist | ||
| */ | ||
| OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException; | ||
|
|
||
| /** | ||
| * Get file status for a file or a directory. | ||
| * | ||
| * @param args the args of the key provided by client. | ||
| * @param clientAddress a hint to key manager, order the datanode in returned | ||
| * pipeline by distance between client and datanode. | ||
| * @return file status. | ||
| * @throws IOException if file or bucket or volume does not exist | ||
| */ | ||
| OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress) | ||
| throws IOException; | ||
|
|
||
| void createDirectory(OmKeyArgs args) throws IOException; | ||
|
|
||
| OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, | ||
|
|
@@ -49,6 +69,21 @@ OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, | |
| */ | ||
| OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress) throws IOException; | ||
|
|
||
| /** | ||
| * List the status for a file or a directory and its contents. | ||
| * | ||
| * @param keyArgs the args of the key provided by client. | ||
| * @param recursive For a directory if true all the descendants of a | ||
| * particular directory are listed | ||
| * @param startKey Key from which listing needs to start. If startKey | ||
| * exists its status is included in the final list. | ||
| * @param numEntries Number of entries to list from the start key | ||
| * @param clientAddress a hint to key manager, order the datanode in returned | ||
| * pipeline by distance between client and datanode. | ||
| * @return list of file status | ||
| * @throws IOException if file or bucket or volume does not exist | ||
| */ | ||
| List<OzoneFileStatus> listStatus(OmKeyArgs keyArgs, boolean recursive, | ||
| String startKey, long numEntries, String clientAddress) | ||
|
There was a problem hiding this comment. In my comment on the previous PR:
I meant both There was a problem hiding this comment. Sure, I will make the changes in my next commit. There was a problem hiding this comment. @adoroszlai , Sorry for re-opening the discussion on this comment. On a second thought There was a problem hiding this comment.
It is also used by several tests, which all pass There was a problem hiding this comment. OK, addressed in my new commit |
||
| throws IOException; | ||
| } | ||
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.
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.
I think
refreshPipelinecondition was removed intentionally in HDDS-3658 and should not be restored. (This is also causing failure ofTestKeyManagerUnit.testLookupFileWithDnFailureunit test.)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.
Thanks @adoroszlai again for the review help.
Yes, you are correct. Test passed locally without the
if (refreshPipeline)condition.Point-1) That means, the caller should call
refreshPipeline(OmKeyInfo value)method without therefreshPipelinecondition.Point-2) Also, I'm planning to move the refreshPipeline call outside BUCKET_LOCK and will be moving to here. Now, its a duplicate call added as part of HDDS-3658.
Does these two changes make sense to you?
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.
Yes, both points make sense. Thanks for noticing the duplicate call.
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.
OK, Done!