-
Notifications
You must be signed in to change notification settings - Fork 581
HDDS-13844. Decouple DirectoryDeletingService delete batching from Ratis request size. #9270
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
Conversation
swamirishi
left a comment
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 patch @aryangupta1998 left some review comments inline
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/DeleteKeysResult.java
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
swamirishi
left a comment
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.
left more review comments
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
|
@yandrey321 do you review the patch? |
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Show resolved
Hide resolved
...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
Outdated
Show resolved
Hide resolved
swamirishi
left a comment
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.
LGTM
If the patch needs to be updated before merge, then please don't approve, since it's confusing. |
swamirishi
left a comment
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.
LGTM @aryangupta1998 thanks for the patch
…tis request size. (apache#9270) (cherry picked from commit 63ee00d) Conflicts: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java Change-Id: If65edcf62195323d437cdb4a5500d82578bc59a0
What changes were proposed in this pull request?
In DirectoryDeletingService, when submitting the request to OM for further processing, we should check if the request exceeds the default rate limit (32 MB). If it does, we should break the request accordingly.
Also, we should process the number of directories to be deleted based on the number specified by the config(ozone.path.deleting.limit.per.task).
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13844
How was this patch tested?
Tested Manually(via UT).