HDDS-16650. Reuse bucket metadata in S3 lifecycle PUT and DELETE by rich7420 · Pull Request #11372 · apache/ozone

rich7420 · 2026-09-30T12:50:54Z

What changes were proposed in this pull request?

Reuse bucket metadata in S3RequestContext for PUT and DELETE lifecycle configuration requests. Owner verification and the subsequent operation share the same OzoneBucket, avoiding a second InfoBucket RPC when expected-owner verification succeeds. This preserves the existing bucket READ requirement, owner validation, bucket layout handling, and error mapping. GET continues to call ClientProtocol directly.

This combines the PUT change from #11374 with the DELETE change, as suggested in review.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16650
https://issues.apache.org/jira/browse/HDDS-16649

How was this patch tested?

Full S3 Gateway unit suite before the bucket-name simplification: 1176 tests, 0 failures or errors.
After the simplification: 118 relevant tests passed; checkstyle passed.
Restoring the original lifecycle handlers causes three bucket-lookup-count cases to fail.
Context tests cover repeated lookup, different-bucket rejection, lookup failure, and isolation between requests.
Checkstyle and RAT passed.
Fork CI and 10×10 flaky-test-check for the PUT, DELETE, and request-context suites on Java 21 are in progress.

…teBucketLifecycle

…ucketLifecycleConfiguration

Copilot

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

Russole · 2026-09-30T13:38:33Z

  }

-  private void verifyBucketOwner(S3RequestContext context, String bucketName) throws OS3Exception {
+  private OzoneBucket verifyBucketOwner(S3RequestContext context, String bucketName) throws OS3Exception {


Could we also reuse the verified bucket in the PUT and GET lifecycle paths to avoid the second getBucket() call?

@Russole Thanks for mention that! The PUT change is already prepared under HDDS-16649. GET was addressed in #11355, which is merged and calls ClientProtocol directly without the extra bucket lookup.

Thanks @rich7420 for the patch. This has overlap with #11374, so there will be conflict for one or the other. It would be nice to create PR only after merge of the other, to reduce both reviewer and CI time. Actually, in this case I think the change is small enough to combine the two PRs.

Thanks, that makes sense. I'll combine the PUT and DELETE changes here and use the request-context approach you suggested in #11374.

Updated in ab3271f: PUT and DELETE now use S3RequestContext#getBucket. Closed #11374 and kept this PR as draft while fork CI runs.

Russole

Thanks @rich7420 for working on this. Left some comments.

adoroszlai

Thanks @rich7420 for updating the patch.

adoroszlai · 2026-10-01T14:01:40Z

+      bucketName = name;
+    } else {
+      Preconditions.assertTrue(bucketName.equals(name), "Multiple buckets in one request are not supported");


minor optimization: bucketName is not needed, since bucket knows its name

Suggested change

bucketName = name;

} else {

Preconditions.assertTrue(bucketName.equals(name), "Multiple buckets in one request are not supported");

} else {

Preconditions.assertEquals(bucket.getName(), name, "Multiple buckets in one request are not supported");

adoroszlai · 2026-10-01T14:01:47Z

  private final EndpointBase endpoint;
  private S3GAction action;
  private OzoneVolume volume;
+  private String bucketName;


Suggested change

private String bucketName;

rich7420 · 2026-10-02T08:27:24Z

@adoroszlai and @Russole thanks for the review!

chungen0126 · 2026-10-02T15:44:58Z

Thanks @rich7420 for working on this, @adoroszlai for the review.

However, This change does not reduce the number of RPCs. The original implementation already caches OzoneBucket, so there wouldn't be a second InfoBucket RPC.

Please verify via distributed tracing after the modification to see if it behaves as expected.

adoroszlai · 2026-10-02T16:19:19Z

change does not reduce the number of RPCs. The original implementation already caches OzoneBucket, so there wouldn't be a second InfoBucket RPC.

@chungen0126 Thanks for checking. Can you please point to the place where OzoneBucket is cached?

rich7420 · 2026-10-02T16:31:41Z

Thanks @chungen0126. I verified this with real S3 Gateway → OM RPCs in a MiniOzoneCluster, using paired client/server traces.

For both lifecycle PUT and DELETE, InfoBucket goes from 2 to 1 with a matching expected-owner header; without it, the count stays at 1. Repeated requests give the same counts. The previous context cached the volume, while each getBucket() still issued an RPC.

Traces, reproducer, and before/after results.

adoroszlai · 2026-10-02T16:40:01Z

I have also verified with compose cluster using bucketlifecycle.robot.

chungen0126 · 2026-10-02T16:43:11Z

Thanks @chungen0126. I verified this with real S3 Gateway → OM RPCs in a MiniOzoneCluster, using paired client/server traces.

For both lifecycle PUT and DELETE, InfoBucket goes from 2 to 1 with a matching expected-owner header; without it, the count stays at 1. Repeated requests give the same counts. The previous context cached the volume, while each getBucket() still issued an RPC.

Traces, reproducer, and before/after results.

You are right, I missed that case. Thanks for the detailed verification and providing the traces!

rich7420 · 2026-10-02T16:44:48Z

Thanks for checking!

adoroszlai · 2026-10-02T16:57:22Z

BTW, it may be easier to check in OM audit log than distributed tracing.

chungen0126 · 2026-10-02T17:00:38Z

change does not reduce the number of RPCs. The original implementation already caches OzoneBucket, so there wouldn't be a second InfoBucket RPC.

@chungen0126 Thanks for checking. Can you please point to the place where OzoneBucket is cached?

My bad. I thought it was called from ObjectRequestContext, but it's actually called from OzoneVolume here.

rich7420 added 2 commits September 30, 2026 14:48

HDDS-16650. Reuse bucket metadata after owner verification in S3 Dele…

8d62d82

…teBucketLifecycle

HDDS-16649. Reuse bucket metadata after owner verification in S3 PutB…

537acdc

…ucketLifecycleConfiguration

Copilot AI balanced review requested due to automatic review settings September 30, 2026 12:50

Copilot AI reviewed Sep 30, 2026

github-actions Bot added the s3 S3 Gateway label Sep 30, 2026

Russole reviewed Sep 30, 2026

View reviewed changes

Gargi-jais11 added the s3-lifecycle HDDS-8342 label Oct 1, 2026

rich7420 added 3 commits October 1, 2026 16:56

Merge remote-tracking branch 'upstream/master' into HDDS-16650

b730db3

Merge branch 'HDDS-16649' into HDDS-16650

d5eec88

HDDS-16650. Reuse request-context bucket for lifecycle PUT and DELETE

ab3271f

rich7420 marked this pull request as draft October 1, 2026 09:04

rich7420 changed the title ~~HDDS-16650. Reuse bucket metadata after owner verification in S3 DeleteBucketLifecycle~~ HDDS-16650. Reuse bucket metadata in S3 lifecycle PUT and DELETE Oct 1, 2026

rich7420 mentioned this pull request Oct 1, 2026

HDDS-16649. Reuse bucket metadata after owner verification in S3 PutBucketLifecycleConfiguration #11374

Closed

adoroszlai reviewed Oct 1, 2026

View reviewed changes

HDDS-16650. Use cached bucket name in request context

8f9343c

adoroszlai marked this pull request as ready for review October 2, 2026 06:26

adoroszlai approved these changes Oct 2, 2026

View reviewed changes

adoroszlai merged commit 037a367 into apache:master Oct 2, 2026
60 checks passed

chungen0126 mentioned this pull request Oct 2, 2026

HDDS-16659. Reuse the destination bucket in the S3 CopyObject datastream path #11383

Merged

Conversation

rich7420 commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Russole Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

rich7420 Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

rich7420 Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

rich7420 Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Russole left a comment

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

adoroszlai Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

adoroszlai Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rich7420 commented Oct 2, 2026

Uh oh!

chungen0126 commented Oct 2, 2026

Uh oh!

adoroszlai commented Oct 2, 2026

Uh oh!

rich7420 commented Oct 2, 2026

Uh oh!

adoroszlai commented Oct 2, 2026

Uh oh!

chungen0126 commented Oct 2, 2026

Uh oh!

rich7420 commented Oct 2, 2026

Uh oh!

adoroszlai commented Oct 2, 2026

Uh oh!

chungen0126 commented Oct 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rich7420 commented Sep 30, 2026 •

edited

Loading

rich7420 Sep 30, 2026 •

edited

Loading