feat(storage): implement internal pre-warmed stream dispatch and caching for BiDi reads#16259
feat(storage): implement internal pre-warmed stream dispatch and caching for BiDi reads#16259kalragauri wants to merge 3 commits into
Conversation
…ing for BiDi reads
There was a problem hiding this comment.
Code Review
This pull request introduces support for an initial read range (pre-warmed cache range) in the asynchronous storage client, allowing the client to skip making a new request on a cache hit. The review feedback highlights a critical use-after-move bug in AsyncConnectionImpl::Open, fragile member initialization order in ObjectDescriptorImpl, and a style guide violation regarding default parameters in an internal namespace, which requires reordering the constructor parameters and updating its callers.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16259 +/- ##
==========================================
- Coverage 92.27% 92.27% -0.01%
==========================================
Files 2214 2214
Lines 206505 206629 +124
==========================================
+ Hits 190556 190667 +111
- Misses 15949 15962 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
511f07a to
91619d4
Compare
91619d4 to
ab05549
Compare
| ->Open({std::move(spec), | ||
| google::cloud::internal::MergeOptions(std::move(opts), | ||
| connection_->options()), | ||
| /*initial_read_range=*/absl::nullopt}) |
There was a problem hiding this comment.
So in all the cases will there be only one read range or we can give multiple read ranges?
There was a problem hiding this comment.
For now, we are only allowing a single range. This optimization targets the 2 round trip penalty of stream setup + read intent. Once the stream is open, subsequent ranges can be multiplexed onto the established stream asynchronously without blocking the initial application threads.
For advanced use cases that require scattered multiplexed reads, ObjectDescriptorImpl already supports assigning multiple concurrent read_ids downstream once the session is opened:
There was a problem hiding this comment.
Yes, I understand that we have feature for sending multiple read requests later, but I think in a request we can send 100 read ranges and we are adding this feature to make our api faster. I just want to know whether sending multiple request with Open would make it more faster or sending one read range would be enough to warm up the stream?
This PR lays the groundwork to support eager streaming for BiDi reads in the GCS async gRPC client. It multiplexes read ranges into the handshake packets and builds the internal object descriptor caching layer.
NOTE: This PR is restricted to internal updates. Public API changes will be introduced in a subsequent PR.
Key Changes