fix(storage): Reset write offset on gRPC BidiWriteObject resumable uploads#16083
fix(storage): Reset write offset on gRPC BidiWriteObject resumable uploads#16083kalragauri wants to merge 2 commits intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the OnQuery method in both AsyncWriterConnectionBuffered and AsyncWriterConnectionResumed to include an is_resume flag, ensuring that write_offset_ is correctly reset to zero upon a resume. Corresponding unit tests have been added to verify this logic. The review feedback highlights a potential integer underflow risk when updating write_offset_ if the server's reported persisted size exceeds the client's current write position, suggesting a defensive check to reset the offset to zero in such scenarios.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16083 +/- ##
========================================
Coverage 92.70% 92.71%
========================================
Files 2351 2351
Lines 217954 218088 +134
========================================
+ Hits 202047 202190 +143
+ Misses 15907 15898 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When a stream breaks, the connection enters a resume state. A key step in OnResume is to create a new underlying connection. This new connection is initialized to start writing data from the persisted size reported by the GCS backend.
The existing logic did not work as expected in some cases because it used relative decrements that became invalid after buffer truncation. This could happen if the size of payload persisted by the server did not match the size sent by the client.
This change introduces an
is_resumeflag to theOnQuerymethod. WhenOnQueryis called as part of theResume()flow (i.e., after a new underlying connection is established),write_offset_is now explicitly set to0. This ensures that theWriteLoopcorrectly starts sending from the beginning of the truncatedresend_buffer_on the resumed stream.