Skip to content

HDDS-13517. Decouple delete batch limits from Ratis request size for KeyDeletingService. - #8874

Merged
swamirishi merged 10 commits into
apache:masterfrom
aryangupta1998:decouple
Aug 25, 2025
Merged

HDDS-13517. Decouple delete batch limits from Ratis request size for KeyDeletingService.#8874
swamirishi merged 10 commits into
apache:masterfrom
aryangupta1998:decouple

Conversation

@aryangupta1998

@aryangupta1998aryangupta1998 commented Jul 29, 2025•
edited by sadanand48
Loading

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

In KeyDeletingService, when submitting the request to SCM and 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.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit test (Aid from Gemini)

sadanand48
sadanand48 reviewed Jul 30, 2025
View reviewed changes
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
@ashishkumar50

ashishkumar50 commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

@swamirishi Can you please help to review this patch.

sumitagrawl
sumitagrawl reviewed Aug 14, 2025
View reviewed changes

@sumitagrawlsumitagrawl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aryangupta1998 Thanks for working over this, given few comments. Additionally, check consistency if one batch success, other fails.

Comment thread...va/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
swamirishi
swamirishi requested changes Aug 17, 2025
View reviewed changes

@swamirishiswamirishi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aryangupta1998 thanks for the patch. I have left a few review comments inline

Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment threadhadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java Outdated
@ashishkumar50ashishkumar50 mentioned this pull request Aug 19, 2025
Merged
sumitagrawl
sumitagrawl reviewed Aug 20, 2025
View reviewed changes

@sumitagrawlsumitagrawl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aryangupta1998 given few more commens

Comment thread...va/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
sumitagrawl
sumitagrawl approved these changes Aug 21, 2025
View reviewed changes

@sumitagrawlsumitagrawl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

swamirishi
swamirishi requested changes Aug 22, 2025
View reviewed changes

@swamirishiswamirishi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aryangupta1998 thanks for updating the patch I have left more minor review comments inline please check

Comment thread...va/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
swamirishi
swamirishi requested changes Aug 24, 2025
View reviewed changes

@swamirishiswamirishi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have few nitpicky changes.

Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java
Comment thread...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java Outdated
swamirishi
swamirishi approved these changes Aug 25, 2025
View reviewed changes

@swamirishiswamirishi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@aryangupta1998 thanks for the patch and promptly addressing all the review comments LGTM.

@swamirishi
swamirishi merged commit fbc393d into apache:masterAug 25, 2025
81 of 82 checks passed
@swamirishi

swamirishi commented Aug 25, 2025

Copy link
Copy Markdown
Contributor

@sadanand48 I have merged this PR if you have any more review comments. Let us do it in another PR.

swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Dec 3, 2025
@aryangupta1998@swamirishi
CDPD-88435. HDDS-13517. Decouple delete batch limits from Ratis reque…
a92b96b
…st size for KeyDeletingService. (apache#8874)
Co-authored-by: Aryan Gupta <aryan.gupta@cloudera.com>
(cherry picked from commit fbc393d)
Conflicts:
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java
Change-Id: Ieaa2ba9e85f4b90e4b4de70a0a21722ef54e0493
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sadanand48sadanand48sadanand48 left review comments
@sumitagrawlsumitagrawlsumitagrawl approved these changes
@swamirishiswamirishiswamirishi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@aryangupta1998@ashishkumar50@swamirishi@sumitagrawl@sadanand48