-
Notifications
You must be signed in to change notification settings - Fork 85
chore: new bidi appendable channel bootstrappable #3226
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
chore: new bidi appendable channel bootstrappable #3226
Conversation
BenWhitehead
commented
Aug 5, 2025
- Add FlushPolicy.MinFlushSizeFlushPolicy.maxPendingBytes
- Add new contructor to ChannelSession to allow the resultFuture to be passed in.
- Create temporary BlobAppendableUploadImpl.AppendableUnbufferedWritableByteChannel to allow both old and new implementations to be passed to BlobAppendableUploadImpl -- this interface will be removed in cleanup
- Add new implementation using streaming buffer management as a sibling to the existing implementation. The existing implementation will be removed entirely in cleanup.
- Refactored tests to follow
555ee59 to
cbee997
Compare
c0febee to
bff6b91
Compare
cbee997 to
e3938f0
Compare
bff6b91 to
287df3a
Compare
e3938f0 to
0df938f
Compare
287df3a to
bff9131
Compare
0df938f to
f262a2c
Compare
* Add FlushPolicy.MinFlushSizeFlushPolicy.maxPendingBytes * Add new contructor to ChannelSession to allow the resultFuture to be passed in. * Create temporary BlobAppendableUploadImpl.AppendableUnbufferedWritableByteChannel to allow both old and new implementations to be passed to BlobAppendableUploadImpl -- this interface will be removed in cleanup * Add new implementation using streaming buffer management as a sibling to the existing implementation. The existing implementation will be removed entirely in cleanup. * Refactored tests to follow
bff9131 to
fb34cb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a little lost on this one, but the general idea seems reasonable
| ? storage.getBidiWriteObjectRequestForTakeover(info, opts) | ||
| : storage.getBidiWriteObjectRequest(info, opts); | ||
| if (newImpl) { | ||
| // TODO: make configurable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there style rules for TODOs? Do they need bug numbers or names or something? Actually, is there a style guide in general I should be checking?
## 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
|
Merged in #3231 |