Skip to content

HDDS-13920. Conditional Reads (GetObject, HeadObject)#10031

Merged
peterxcli merged 8 commits into
apache:masterfrom
peterxcli:HDDS-13920-conditional-reads
Apr 10, 2026
Merged

HDDS-13920. Conditional Reads (GetObject, HeadObject)#10031
peterxcli merged 8 commits into
apache:masterfrom
peterxcli:HDDS-13920-conditional-reads

Conversation

@peterxcli

@peterxcli peterxcli commented Apr 2, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Implement S3 Conditional Reads for Ozone and introduce S3ConditionalRequest class to centralized conditional request logic.

Only s3gateway involve for Conditional Reads.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit test, S3 SDK test

Add tests for If-Match, If-None-Match, If-Modified-Since, and If-Unmodified-Since headers in both GET and HEAD requests. Enhance response handling for conditional requests in ObjectEndpoint.
@peterxcli peterxcli self-assigned this Apr 7, 2026

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

@peterxcli Thanks for the patch, left some comments.

/**
* Shared parsing and evaluation for S3 conditional request headers.
*/
final class S3ConditionalRequest {

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.

Is the conditional request logic can be implemented as ObjectOperationHandler (the chain of responsibility pattern)? I'm OK to leave it be if it's too tedious.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

let me take a look.

@peterxcli peterxcli requested a review from ivandika3 April 9, 2026 19:09

@ivandika3 ivandika3 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 @peterxcli , LGTM +1.

This can be merged after merge conflicts are resolved.

@peterxcli peterxcli added the s3 S3 Gateway label Apr 10, 2026
@peterxcli peterxcli merged commit 675410e into apache:master Apr 10, 2026
45 checks passed
@peterxcli peterxcli deleted the HDDS-13920-conditional-reads branch April 10, 2026 06:39
@peterxcli

Copy link
Copy Markdown
Member Author

Thanks @ivandika3 for the review!

@peterxcli

Copy link
Copy Markdown
Member Author

https://x.com/Sirupsen/status/2050895383866249618
image

we could also test our conditional requests perf.

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.

2 participants