Skip to content

fix(storage): preserve flush state across concurrent flushes in async writers#16274

Open
kalragauri wants to merge 1 commit into
googleapis:mainfrom
kalragauri:feat/flush-fix
Open

fix(storage): preserve flush state across concurrent flushes in async writers#16274
kalragauri wants to merge 1 commit into
googleapis:mainfrom
kalragauri:feat/flush-fix

Conversation

@kalragauri

Copy link
Copy Markdown
Contributor

In SetFlushed(), both AsyncWriterConnectionBufferedState and AsyncWriterConnectionResumedState were unconditionally resetting the internal state flag flush_ = false at the very start of the method when a flush succeeded. If a caller issued multiple concurrent Flush() requests (or queued a new Flush() while a previous Flush() was in-flight), resetting flush_ = false prematurely caused the background WriteLoop to treat any remaining queued data as buffered data rather than explicit flush requests.

This PR modifies SetFlushed() to defer resetting flush_ = false until pending_flush_promises_.empty() evaluates to true (or when handling empty promise edge cases). This guarantees that as long as there are pending flush promises in the queue, flush_ remains true, instructing the WriteLoop to continue invoking flush operations on subsequent chunks until all queued flush promises are satisfied.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jul 24, 2026

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

Copy link
Copy Markdown

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 modifies the SetFlushed method in both AsyncWriterConnectionBufferedState and AsyncWriterConnectionResumedState to only reset the flush_ flag to false when there are no remaining pending flush promises. The unit tests have been updated to align with this change, expecting a second flush and query sequence. I have no feedback to provide as there are no review comments.

@kalragauri
kalragauri marked this pull request as ready for review July 24, 2026 09:57
@kalragauri
kalragauri requested review from a team as code owners July 24, 2026 09:57
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.29%. Comparing base (a9d896c) to head (1586e25).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #16274   +/-   ##
=======================================
  Coverage   92.29%   92.29%           
=======================================
  Files        2221     2221           
  Lines      207372   207384   +12     
=======================================
+ Hits       191386   191399   +13     
+ Misses      15986    15985    -1     

☔ View full report in Codecov by Harness.
📢 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.

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