Skip to content

Conversation

BenWhitehead
Copy link
Collaborator

This will have tests added in a followup commit

@BenWhitehead BenWhitehead requested a review from a team as a code owner August 5, 2025 19:29
@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 Aug 5, 2025
@BenWhitehead BenWhitehead force-pushed the nonblocking-appendable/07/stream branch from f3d0fff to a69bad6 Compare August 5, 2025 19:54
@BenWhitehead BenWhitehead requested a review from a team as a code owner August 5, 2025 19:54
@BenWhitehead BenWhitehead force-pushed the nonblocking-appendable/08/unbuffered-channel branch from 0858e8a to 41da934 Compare August 5, 2025 19:54
@BenWhitehead BenWhitehead force-pushed the nonblocking-appendable/07/stream branch from a69bad6 to 4d96b53 Compare August 5, 2025 20:42
@BenWhitehead BenWhitehead force-pushed the nonblocking-appendable/08/unbuffered-channel branch 2 times, most recently from 90bddd1 to bd3e81f Compare August 8, 2025 20:54
@BenWhitehead BenWhitehead force-pushed the nonblocking-appendable/07/stream branch from 4d96b53 to 365bce5 Compare August 11, 2025 19:38
This will have tests added in a followup commit
@BenWhitehead BenWhitehead force-pushed the nonblocking-appendable/08/unbuffered-channel branch from bd3e81f to 6d24a64 Compare August 11, 2025 19:38
}

@Override
public long writeAndClose(ByteBuffer[] srcs, int offset, int length) throws IOException {

Choose a reason for hiding this comment

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

Why isn't this just

long written = write(srcs, offset, length);
close();
return written;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The internalWrite and write methods are both non-blocking and may not consume all of the bytes present in srcs. However, close does need to block. So we loop here to only call close once all of the bytes have been successfully enqueued. Then close can do it's normal blocking waiting for the necessary ack from gcs.

BenWhitehead added a commit that referenced this pull request Aug 20, 2025
## Description
feat: *breaking behavior* rewrite Storage.blobAppendableUpload to be non-blocking and have improved throughput (#3231)

Rewrite internals of BlobAppendableUpload to provide non-blocking write calls, and it take advantage of grpc async message handling.

When `AppendableUploadWriteableByteChannel#write(ByteBuffer)` is called, an attempt will be made to enqueue the bytes in the outbound queue to GCS.
If there is only enough room to partially consume the bytes provided in the `ByteBuffer` the write call will return early specifying the number of bytes actually consumed.

As acknowledgements come in from gcs, enqueued messages will be evicted freeing space in the outbound queue. Thereby allowing more bytes to be consumed and enqueued.

Given appendable objects are still in private preview I can't quote any metrics here, however preliminary benchmarking of several million objects across a range of sizes show across the board throughput improvments.

Because the channel's write call is now non-blocking, if you want to block your application until the full buffer is consumed some new helper methods have been added in StorageChannelUtils to provide blocking behavior.

A new method `MinFlushSizeFlushPolicy#withMaxPendingBytes(long)` has been added to allow limiting the number of pending outbound bytes. The default values is 16MiB, but can be configured lower if necessary.

## Release Notes

BEGIN_COMMIT_OVERRIDE

BEGIN_NESTED_COMMIT
feat: *breaking behavior* rewrite Storage.blobAppendableUpload to be non-blocking and have improved throughput (#3231)
END_NESTED_COMMIT

BEGIN_NESTED_COMMIT
feat: add StorageChannelUtils to provide helper methods to perform blocking read/write to/from non-blocking channels (#3231)
END_NESTED_COMMIT

BEGIN_NESTED_COMMIT
feat: add MinFlushSizeFlushPolicy#withMaxPendingBytes(long) (#3231)
END_NESTED_COMMIT

BEGIN_NESTED_COMMIT
fix: update BlobAppendableUploadConfig and FlushPolicy.MinFlushSizeFlushPolicy to default to 4MiB minFlushSize and 16MiB maxPendingBytes (#3249)
END_NESTED_COMMIT

BEGIN_NESTED_COMMIT
fix: make FlushPolicy${Min,Max}FlushSizeFlushPolicy constructors private (#3217)
END_NESTED_COMMIT

END_COMMIMT_OVERRIDE

## Sub PRs
This PR is made of up the following PRs, in sequence
1. #3217
2. #3218 
3. #3219
4. #3220
5. #3221
6. #3222
7. #3223
8. #3224 
9. #3225 
10. #3226 
11. #3227 
12. #3228 
13. #3229 
14. #3230 
15. #3235 
16. #3236 
17. #3241
18. #3242
19. #3246
20. #3248
21. #3249
22. #3252
@BenWhitehead
Copy link
Collaborator Author

Merged in #3231

@BenWhitehead BenWhitehead deleted the nonblocking-appendable/08/unbuffered-channel branch August 20, 2025 21:13
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. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants