fix(storage): always use latest write handle for fast resume in gRPC BidiWriteObject #15795
Conversation
shubham-up-47
had a problem deploying
to
external
GitHub Actions
Error
product-auto-label
bot
added
the
api: storage
Summary of ChangesHello @shubham-up-47, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness and efficiency of asynchronous object writing in the C++ SDK by ensuring that the most current write handle is always utilized during stream resumptions. This critical update aligns with the gRPC protocol, leading to more stable and performant data transfers, especially in scenarios requiring stream recovery. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to improve fast resume behavior for gRPC BidiWriteObject streams by always using the latest write_handle. The changes correctly propagate the write_handle for resumed streams and introduce it for continued streams. However, I've found a couple of issues with the implementation. There appears to be a compilation error in writer_connection_resumed.cc due to incorrect member access. Additionally, the logic in writer_connection_impl.cc for continued streams seems to only work for the first write request, but not for subsequent ones, due to how the request object is cleared. I've also suggested an improvement to the new test case to cover this scenario. Addressing these points will ensure the feature works as intended.
google/cloud/storage/internal/async/writer_connection_resumed.cc
Outdated
Show resolved
Hide resolved
google/cloud/storage/internal/async/writer_connection_impl_test.cc
Outdated
Show resolved
Hide resolved
shubham-up-47
changed the title
shubham-up-47
had a problem deploying
to
external
GitHub Actions
Error
shubham-up-47
had a problem deploying
to
external
GitHub Actions
Error
shubham-up-47
had a problem deploying
to
external
GitHub Actions
Error
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15795 +/- ##
========================================
Coverage 92.94% 92.94%
========================================
Files 2457 2458 +1
Lines 227128 227548 +420
========================================
+ Hits 211099 211500 +401
- Misses 16029 16048 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
shubham-up-47
dismissed stale reviews from diegomarquezp and v-pratap
via
31c1d3a
shubham-up-47
requested a deployment
to
external
GitHub Actions
Waiting
|
/gcbrun |
3 participants
This PR updates the C++ SDK to always use the latest write_handle received from any BidiWriteObjectResponse when resuming or continuing gRPC BidiWriteObject streams. This ensures fast resume behavior, reduces latency, and avoids offset mismatch errors by following the protocol requirement to propagate the most recent write handle.
Testing the changes,