Skip to content

fix(storage): use server-reported size on stream reconnect to avoid duplicate writes#16115

Open
v-pratap wants to merge 3 commits into
googleapis:mainfrom
v-pratap:more-data-in-write
Open

fix(storage): use server-reported size on stream reconnect to avoid duplicate writes#16115
v-pratap wants to merge 3 commits into
googleapis:mainfrom
v-pratap:more-data-in-write

Conversation

@v-pratap
Copy link
Copy Markdown
Contributor

Problem: When the C++ SDK loses connection while writing to an appendable (Rapid) object and tries to reconnect, it sometimes sends data that the server has already received. This happens because the client was ignoring the current object size reported by the server during the reconnect, and instead relied on its own older progress information saved before the connection dropped. This caused the server to reject the writes as out-of-range.

Solution: Modified the client to check the server's response immediately after reconnecting. If the server returns the current size of the object (and a handle to continue writing), the client now uses that size as the starting offset for sending new data. This prevents the client from re-sending data that the server already has.

@v-pratap v-pratap requested review from a team as code owners May 26, 2026 08:50
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label May 26, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the AsyncWriterConnectionResumedState class to extract the persisted offset directly from the first response's resource size or persisted size when resuming a write operation, falling back to the persisted state if neither is present. A corresponding unit test has also been added to verify this behavior. The review feedback suggests optimizing performance by using std::move to avoid copying the protobuf resource message when finalizing the object.

Comment thread google/cloud/storage/internal/async/writer_connection_resumed.cc Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 92.55319% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.71%. Comparing base (77109ef) to head (ddfdfd6).

Files with missing lines Patch % Lines
...torage/internal/async/writer_connection_resumed.cc 50.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16115      +/-   ##
==========================================
- Coverage   92.71%   92.71%   -0.01%     
==========================================
  Files        2353     2353              
  Lines      219274   219364      +90     
==========================================
+ Hits       203303   203386      +83     
- Misses      15971    15978       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@v-pratap v-pratap force-pushed the more-data-in-write branch from 8c001b7 to ddfdfd6 Compare May 27, 2026 06:24
@v-pratap v-pratap requested a review from kalragauri May 27, 2026 08:42
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 Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant