Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update ApiaryUnbufferedWritableByteChannel to be graceful of non-quantum aligned write calls #2493

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

BenWhitehead
Copy link
Collaborator

Despite GCS only allowing incremental updates to a resumable session at 256KiB byte boundaries, we have observed an extremely rare case (<0.000001%) of an incremental write being non-quantum aligned. The change in this PR makes ApiaryUnbufferedWritableByteChannel graceful to this possibility, and will only set the finalization header when close is invoked.

If a write is not quantum aligned, it will either: 1) not be consumed at all, at which point write can be called again with the still enqueued bytes 2) partially consumed, with the position of the provided ByteBuffers updated to reflect how much of their bytes were consumed, matching up with the number of bytes actually consumed returned from write()

Add new integration test to intentionally perform non-quantum aligned write() calls to ApiaryUnbufferedWritableByteChannel.

Separate change to gRPC affected code path to come in a later PR.

b/330550326

…-quantum aligned write calls

Despite GCS only allowing incremental updates to a resumable session at 256KiB byte boundaries, we have observed an extremely rare case of an incremental write being non-quantum aligned. The change in this PR makes ApiaryUnbufferedWritableByteChannel graceful to this possibility, and will only set the finalization header when close is invoked.

If a write is not quantum aligned, it will either:
1) not be consumed at all, at which point write can be called again with the still enqueued bytes
2) partially consumed, with the position of the provided ByteBuffers updated to reflect how much of their bytes were consumed, matching up with the number of bytes actually consumed returned from `write()`

Add new integration test to intentionally perform non-quantum aligned `write()` calls to ApiaryUnbufferedWritableByteChannel.

Separate change to gRPC affected code path to come in a later PR.

b/330550326
@BenWhitehead BenWhitehead added the owlbot:ignore instruct owl-bot to ignore a PR label Apr 8, 2024
@BenWhitehead BenWhitehead requested a review from a team as a code owner April 8, 2024 22:04
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: storage Issues related to the googleapis/java-storage API. labels Apr 8, 2024
* committed.
*/
@Test
public void scenario9() throws Exception {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test would verify that GCS ack'ing fewer bytes than were sent was correctly categorized as a failure scenario, however as we are now gracefully handling the partial write case. We no longer need this test for failure categorization.

@BenWhitehead BenWhitehead merged commit f548335 into main Apr 9, 2024
22 checks passed
@BenWhitehead BenWhitehead deleted the dangling-bytes branch April 9, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. owlbot:ignore instruct owl-bot to ignore a PR size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants