fix(buffers): recompute disk_v2 buffer size on restart to prevent underflow#25767
Conversation
…erflow The disk_v2 ledger previously seeded `total_buffer_size` at load from the sum of the on-disk data file sizes and then had the reader draw it down record-by-record while seeking to its persisted read position. Those two inputs were captured at different moments -- the data files on disk versus the read position in the ledger, which are flushed independently -- so a crash between their flushes could make the draw-down subtract more than the seed held. Because the counter is unsigned, the subtraction wrapped to a near-max value, making the buffer look permanently full and wedging the writer (no further writes, and recovery could stall). The reader now recomputes the unread total authoritatively at the end of its startup seek, from a single consistent snapshot: the total size of the data files still on disk minus the bytes consumed reaching the resume position. By then the seek has deleted every fully-read file, so the survivors are exactly the resume file plus later unread files, and the consumed prefix can never exceed their total -- the counter can no longer underflow across a restart. `update_buffer_size` is removed (its only remaining job, the "buffer reopens with pre-existing records" assertion, moves into the seek where the on-disk total is already computed). Init-time incremental size mutations are dropped in favor of the single post-seek store. (cherry picked from commit 0f12939)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6819630204
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
| } | ||
|
|
||
| let unread_buffer_size = total_data_file_size.saturating_sub(self.bytes_read); |
There was a problem hiding this comment.
When startup hits a bad read after consuming a valid prefix of the current file, next() calls roll_to_next_data_file(), which resets self.bytes_read before seek_to_next_record() reaches this recompute. The skipped file still exists and is included by total_data_file_size(), so this subtraction treats the already-consumed/acknowledged prefix as unread; on buffers near max_size, the reopened writer can block on the inflated size while the reader is waiting for the writer to create the next file.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes a buffer-size accounting underflow in the
disk_v2buffer on restart. The recomputed total buffer size could underflow, making the buffer appear permanently full and wedging the writer. This recomputes the size correctly on restart to prevent the underflow.How did you test this PR?
cargo check -p vector-buffers --tests, plus the added invariants regression coverage. Also exercised by the crash-injecting Antithesis suite.Change Type
Is this a breaking change?
Does this PR include user facing changes?