-
Notifications
You must be signed in to change notification settings - Fork 9k
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
HADOOP-19474: [ABFS][FnsOverBlob] Listing Optimizations to avoid multiple iteration over list response. #7421
base: trunk
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
This comment was marked as outdated.
This comment was marked as outdated.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
* limitations under the License. | ||
*/ | ||
|
||
package org.apache.hadoop.fs.azurebfs.contracts.services; |
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.
Is this class expected to be in the contracts folder ?
throw new AbfsDriverException(e); | ||
} | ||
} catch (IOException e) { | ||
LOG.error("Unable to deserialize list results", e); |
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.
Given we have the uri now, should we include that in the error log as well ?
return listResponseData; | ||
} | ||
|
||
private boolean isRenamePendingJsonPathEntry(BlobListResultEntrySchema entry) { |
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.
missing javadocs
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.
Can be simplified to this :- private boolean isRenamePendingJsonPathEntry(BlobListResultEntrySchema entry) {
String path = entry.path() != null ? entry.path().toUri().getPath() : null;
return path != null && !entry.path().isRoot() && isAtomicRenameKey(path) && path.endsWith(RenameAtomicity.SUFFIX);
}
Description of PR
On blob endpoint, there are a couple of handling that is needed to be done on client side.
This involves:
Currently all three are done in a separate iteration over whole list. This is to pbring all those things to a common place so that single iteration over list reposne can handle all three.
How was this patch tested?
For code changes: