fix(storage): track persisted byte offset for async writer flush promises - #16417
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates both AsyncWriterConnectionBufferedState and AsyncWriterConnectionResumedState to track outstanding Flush() promises alongside their stream target offsets using a new PendingFlush struct. This ensures that multiple queued flush operations are only completed when the server's persisted size reaches or exceeds their respective target offsets. Unit and integration tests have been added to verify this behavior. Feedback is provided regarding a compilation error in writer_connection_resumed.cc where persisted_size (a StatusOr<std::int64_t>) is passed directly to SetFlushed which expects std::int64_t; it is recommended to explicitly check persisted_size.ok() before extracting the value.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16417 +/- ##
==========================================
+ Coverage 92.26% 92.28% +0.02%
==========================================
Files 2246 2246
Lines 212734 212774 +40
==========================================
+ Hits 196273 196366 +93
+ Misses 16461 16408 -53 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
In
AsyncWriterConnectionBufferedStateandAsyncWriterConnectionResumedState,SetFlushed()completed pendingFlush()promises using a simple FIFOpop_front(). Because server-sidepersisted_sizeconfirmations are cumulative byte offsets rather than 1:1 request/response tokens, a subsequentFlush()could be prematurely satisfied by an earlier acknowledgment while its buffered writes were still in-flight.This PR does the following:
PendingFlushstruct to pair eachFlush()promise with its expectedtarget_offset(buffer_offset_ + resend_buffer_.size()).SetFlushed()to dequeue and satisfy only promises whosetarget_offset <= persisted_size.