Skip to content

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

Closed
rich7420 wants to merge 1 commit into
apache:masterfrom
rich7420:HDDS-16649
Closed

rich7420 wants to merge 1 commit into
apache:masterfrom
rich7420:HDDS-16649

Conversation

@rich7420

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Reuse the OzoneBucket fetched during expected bucket owner verification in PutBucketLifecycleConfiguration. Requests that pass verification currently fetch the same bucket again before constructing and setting the lifecycle configuration. Reusing it removes one InfoBucket RPC while retaining the existing bucket READ requirement, owner validation, bucket layout handling, and error handling. Requests without a non-empty expected-owner header still perform the existing lookup.

What is the link to the Apache JIRA

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

How was this patch tested?

Copilot AI balanced review requested due to automatic review settings September 30, 2026 15:01

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.

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

@priyeshkaratha priyeshkaratha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @rich7420 for the patch. Changes LGTM

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

Thanks @rich7420 for the patch.

Comment on lines +133 to +137
OzoneBucket ozoneBucket = verifyBucketOwner(context, bucketName);
S3LifecycleConfiguration s3LifecycleConfiguration;
OzoneBucket ozoneBucket = context.getVolume().getBucket(bucketName);
if (ozoneBucket == null) {
ozoneBucket = context.getVolume().getBucket(bucketName);
}

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.

Instead of changing verifyBucketOwner to return the bucket and handling null case here, I would like to suggest a different approach:

  • add OzoneBucket getBucket(String bucketName) in S3RequestContext, storing the bucket, similar to getVolume(); I think each request applies to only one bucket, so it can reject subsequent getBucket(differentName) calls for now
  • use context.getBucket(bucketName) in both verifyBucketOwner and here

@rich7420

rich7420 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Combined into #11372, which now covers both HDDS-16649 and HDDS-16650 using the request-context bucket cache suggested in review.

@rich7420 rich7420 closed this Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

s3 S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants