Skip to content
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-4232. Use single thread for KeyDeletingService. #1415

Merged
merged 2 commits into from Sep 29, 2020

Conversation

lokeshj1703
Copy link
Contributor

@lokeshj1703 lokeshj1703 commented Sep 10, 2020

What changes were proposed in this pull request?

KeyDeletingService scan the keys from a particular rocksdb table and sends deletion request to SCM. Every thread would scan the table and send deletion requests. This can lead to multiple deletion request for a particular block. There is currently no way to distribute the keys to be deleted amongst multiple threads.

What is the link to the Apache JIRA

HDDS-4232

How was this patch tested?

This lead to failure in TestBlockDeletion. Please refer 49b610a

@lokeshj1703 lokeshj1703 self-assigned this Sep 10, 2020
@@ -66,9 +66,6 @@
private static final Logger LOG =
LoggerFactory.getLogger(KeyDeletingService.class);

// The thread pool size for key deleting service.
private final static int KEY_DELETING_CORE_POOL_SIZE = 2;
Copy link
Contributor

@amaliujia amaliujia Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just change 2 to 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed it in latest commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bharatviswa504
Copy link
Contributor

bharatviswa504 commented Sep 10, 2020

Hi @lokeshj1703
HDDS-4120 This Jira changes the behavior of openKeyCleanup service, and does not call SCM directly, it moves the keys to deleteTable by making a write request to OM. And later it will be picked by KeyDeletingService and it will be only the one to send delete keys request to SCM from OM.

Do you think, we need this change still even after this also for openKeyCleanup?

@lokeshj1703
Copy link
Contributor Author

/pending

Copy link

@github-actions github-actions bot left a 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...)

@lokeshj1703
Copy link
Contributor Author

The PR requires change in 1414 to address the test failure.

@lokeshj1703
Copy link
Contributor Author

HDDS-4120 This Jira changes the behavior of openKeyCleanup service, and does not call SCM directly, it moves the keys to deleteTable by making a write request to OM. And later it will be picked by KeyDeletingService and it will be only the one to send delete keys request to SCM from OM.

@bharatviswa504 Thanks for providing the info! In that case we do not need the changes for openKeyCleanupService. I have addressed it in latest commit.

@lokeshj1703 lokeshj1703 changed the title HDDS-4232. Use single thread for KeyDeletingService and OpenKeyCleanupService. HDDS-4232. Use single thread for KeyDeletingService. Sep 11, 2020
@lokeshj1703
Copy link
Contributor Author

/ready

@github-actions github-actions bot dismissed their stale review September 29, 2020 09:09

Blocking review request is removed.

@github-actions github-actions bot removed the pending label Sep 29, 2020
@adoroszlai
Copy link
Contributor

Thanks @lokeshj1703 for the fix and @amaliujia and @bharatviswa504 for the review.

@adoroszlai adoroszlai merged commit 34f3b91 into apache:master Sep 29, 2020
errose28 pushed a commit to errose28/ozone that referenced this pull request Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants