-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-11808. Key deletion service should support multiple threads #7486
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
ashishkumar50
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.
@aryangupta1998, Thanks for working on this, please find few comments inline.
| () -> { | ||
| try { | ||
| return keyManager.getPendingDeletionKeys(Integer.MAX_VALUE) | ||
| return keyManager.getPendingDeletionKeys(Integer.MAX_VALUE, keyDeletingService.getDeletedKeySupplier()) |
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.
Tests will fail because now same iterator is used by background service and test code. Need to make sure only one is running at a time to get correct result.
| try { | ||
| deletedKeySupplier.reInitItr(); | ||
| } catch (IOException ex) { | ||
| LOG.error("Unable to get the iterator.", ex); |
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.
Return from here if there is an exception, no point in continuing.
17e66a0 to
15f7796
Compare
|
/pending conflicts |
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.
Marking this issue as un-mergeable as requested.
Please use /ready comment when it's resolved.
Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)
conflicts
|
Thank you very much for the patch. I am closing this PR temporarily as there was no activity recently and it is waiting for response from its author. It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time. It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs. If you need ANY help to finish this PR, please contact the community on the mailing list or the slack channel." |
What changes were proposed in this pull request?
When a large amount of data is deleted, we may see slow progress in clearing it out due to the large backlog of pending deletes, even if all deletion services run for their full intervals and collect their maximum number of entries. To speed up deletion further, more threads will need to be configured. This PR helps to support multi-threaded deletion for the key deleting service.
Approach:
By default, we have configured 10 threads. We have a key supplier class with a synchronized get() function, which each thread calls to get a set of key info and then process it. Concurrently, threads call this get() and process the keys until the key limit per task is reached.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11808
How was this patch tested?
Tested Manually.