Skip to content

HDDS-16282. Avoid O(n^2) key removal in S3 DeleteObjects - #11126

Open
rjgoyln wants to merge 1 commit into
apache:masterfrom
rjgoyln:HDDS-16282
Open

HDDS-16282. Avoid O(n^2) key removal in S3 DeleteObjects#11126
rjgoyln wants to merge 1 commit into
apache:masterfrom
rjgoyln:HDDS-16282

Conversation

@rjgoyln

@rjgoyln rjgoyln commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

A multi-object DELETE carries up to 1000 keys, and the failedDeletes audit list was derived by removing every successfully deleted key from the request list — quadratic in the size of the request. Building the failure list directly leaves the response and the audit output unchanged.

Timing that loop in isolation at the 1000-key cap, median of 2000 iterations after warmup on JDK 25:

failed keys before after
0 33 µs 1.2 µs
1 36 µs 1.9 µs
100 150 µs 2.9 µs
500 659 µs 5.1 µs
900 239 µs 4.2 µs

The peak sits near a 50% failure rate rather than at either extreme, because failed keys accumulate at the head of the list and every later remove(Object) scans past them. Against the OM round trip the saving is small; what it buys is dropping a quadratic term from a capped path.

What is the link to the Apache JIRA

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

How was this patch tested?

Two new unit tests pin the response and the failedDeletes audit parameter, for a mixed success/failure request and for a batch delete that fails outright. Both also pass against the previous implementation — that is what they are for. The full ozone-s3gateway suite (760 tests) and checkstyle pass locally.

Generated-by: Claude Code (Opus 5)

A multi-object DELETE may carry up to 1000 keys, and the audit list of
failed deletions was derived by removing every successfully deleted key
from an ArrayList, making the response loop quadratic in the number of
keys.
Copilot AI lite review requested due to automatic review settings August 26, 2026 16:28

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@yandrey321

Copy link
Copy Markdown
Contributor

is there a benchmark that shows performance improvement for these changes?

@rjgoyln

rjgoyln commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

is there a benchmark that shows performance improvement for these changes?

Yes, I've updated the PR body to include the benchmark results and performance details. Please take a look! Thank you.

@yandrey321 yandrey321 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

@rjgoyln
rjgoyln marked this pull request as ready for review August 27, 2026 05:33

@rich7420 rich7420 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.

LG, thanks for the patch @rjgoyln

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