fix(buffers): publish disk buffer progress before notifying readers#25872
fix(buffers): publish disk buffer progress before notifying readers#25872graphcareful wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdbbd6e0f9
ℹ️ 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".
b1cea78 to
c390fab
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Ensure disk buffer readers are notified only after flushed writer progress has been published to the shared ledger.
This consolidates record ID, buffer size, and usage accounting with reader notification for both explicit and implicit flushes. It prevents readers from consuming and acknowledging newly readable bytes while the ledger still contains stale writer state, which could corrupt accounting and stall buffer progress.
Vector configuration
No new configuration is introduced. The change applies to existing disk-buffered components, for example:
How did you test this PR?
Using new antithesis runs located in the branch https://github.com/graphcareful/vector/tree/test/disk-buffer-oversized-progress two assertions were firing:
After discovering the root cause had nothing to do with oversized records, it was tracked down to the final batch remaining stuck in the disk buffer.
With this change the two antithesis failures no longer fire.
cargo test -p vector-buffers --no-default-features(77 passed, 2 pre-existing ignored)cargo test -p vector-buffers publishing_writer_progress_updates_ledger_before_waking_reader --no-default-featuresChange Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
None.