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-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. #7365

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

sadanand48
Copy link
Contributor

What changes were proposed in this pull request?

Currently we limit the number of deletion objects (directories) in each run of DirectoryDeletingService based on Ratis request limit i.e If Ratis request proto size exceeds the limit , DDS stops sending further dirs for purging in the same run which means the task limit per run is not honoured. This PR fixes this by batching the requests as per Ratis proto limit and sending mutiple purge requests in the same run. This way the task limit per run is honoured.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11508

How was this patch tested?

Unit tests

@errose28
Copy link
Contributor

errose28 commented Oct 28, 2024

Thanks for working on this @sadanand48. Are you planning to handle this for SCM block deletion and other OM services in another PR? In that case maybe we should file the PRs under sub-jiras of the original HDDS-11508 Jira for easier tracking.

@sadanand48
Copy link
Contributor Author

Are you planning to handle this for SCM block deletion and other OM services

Sure, will create sub tasks for these when the current one is fixed.

@errose28
Copy link
Contributor

Sounds good, I think that will help with reviews too. In that case, let's use a new Jira for this PR and resolve HDDS-11508 once all the subtasks and their PRs are complete.

@sadanand48 sadanand48 marked this pull request as draft November 15, 2024 08:40
@sadanand48 sadanand48 marked this pull request as ready for review December 6, 2024 08:23
@sadanand48 sadanand48 requested review from sumitagrawl and removed request for sumitagrawl December 9, 2024 07:02
@adoroszlai
Copy link
Contributor

@sadanand48 HDDS-11605 introduced some conflicts, can you please update the PR?

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@sadanand48 Thanks for working over this, have few minor comments...

@sadanand48 sadanand48 marked this pull request as draft January 2, 2025 08:00
@sadanand48
Copy link
Contributor Author

remove the limit case and use only ratis size.

@sumitagrawl @errose28 Could you please review this ? I have implemented the above suggestion and only used ratis size as the limit for sending a delete request.

@sadanand48 sadanand48 marked this pull request as ready for review January 7, 2025 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants