Skip to content

Added error field to block to throw exception early#333

Merged
ozkoca merged 2 commits intoawslabs:physical-io-featurefrom
ozkoca:physical-io-feature-etag-change
Jul 28, 2025
Merged

Added error field to block to throw exception early#333
ozkoca merged 2 commits intoawslabs:physical-io-featurefrom
ozkoca:physical-io-feature-etag-change

Conversation

@ozkoca
Copy link
Collaborator

@ozkoca ozkoca commented Jul 28, 2025

Description of change

This PR supports Block.read method to throw Exception without retry when SDK throws Status Code: 412 exception, when etag of a file changes.

Relevant issues

#325

Does this contribution introduce any breaking changes to the existing APIs or behaviors?

No

Does this contribution introduce any new public APIs or behaviors?

No

How was the contribution tested?

Unit tests, integration tests

Does this contribution need a changelog entry?

N/A


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

Copy link
Collaborator

@fuatbasik fuatbasik left a comment

Choose a reason for hiding this comment

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

thanks @ozkoca . Approved with 2 nits

// This value should be Default ReadAhead bytes or read buffer size whichever
// is higher. Currently, default read buffer size is 128KB, this value
// should be 128KB
private static final int DEFAULT_READ_BYTES = 128 * ONE_KB;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which means default read-ahead bytes would be 128KB min. right? Should we align these fields and call it min. read size maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. Renamed as suggested

public int read(long pos) throws IOException {
Preconditions.checkArgument(0 <= pos, "`pos` must not be negative");
awaitDataWithRetry();
if (error != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we move this check to awaitData?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we move this check to awaitData, library will keep retrying due to the retry strategy. With existing, when there is an error, we will throw exception without retry

@ozkoca ozkoca merged commit 21b1ab0 into awslabs:physical-io-feature Jul 28, 2025
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.

2 participants