fix: update ApiaryUnbufferedWritableByteChannel to be graceful of non-quantum aligned write calls #2493
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-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
commented
Apr 8, 2024
| * committed. | ||
| */ | ||
| @Test | ||
| public void scenario9() throws Exception { |
Collaborator
Author
There was a problem hiding this comment.
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.
frankyn
reviewed
Apr 9, 2024
...loud-storage/src/main/java/com/google/cloud/storage/ApiaryUnbufferedWritableByteChannel.java
Show resolved
Hide resolved
...le-cloud-storage/src/test/java/com/google/cloud/storage/ITUnbufferedResumableUploadTest.java
Show resolved
Hide resolved
frankyn
approved these changes
Apr 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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