-
Notifications
You must be signed in to change notification settings - Fork 490
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
HDDS-11238. Converge redundant getBucket calls for FileSystem client delete #6991
Conversation
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 @tanvipenumudy for the patch. Some improvements suggested.
...zone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
Outdated
Show resolved
Hide resolved
...zone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
Outdated
Show resolved
Hide resolved
...zone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
Outdated
Show resolved
Hide resolved
...zone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
Outdated
Show resolved
Hide resolved
…/ozone/BasicRootedOzoneFileSystem.java Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>
…/ozone/BasicRootedOzoneFileSystem.java Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>
…/ozone/BasicRootedOzoneFileSystem.java Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>
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 for the changes @tanvipenumudy overall the patch looks good.
Can you please address the one comment I have given.
...zone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
Show resolved
Hide resolved
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 for updating the patch @tanvipenumudy
LGTM
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 @tanvipenumudy for updating the patch, LGTM.
Thanks @adoroszlai and @ArafatKhan2198 for reviewing the patch. |
What changes were proposed in this pull request?
A few internal calls as part of FileSystem client delete involve repeated computation of getBucket(). Instead, we may precompute the OzoneBucket object and then pass it onto other dependent calls as they again compute the same object internally.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11238
How was this patch tested?
Existing unit and integration tests should cover the changes