Repository navigation
Conversation
The memory -> disk overflow receiver polls the disk reader inside select! against the memory stage. A record larger than the reader's 256 KiB read buffer needs several fill_buf calls; cancelling the read between them lost the bytes already consumed, so the reader resynchronised on garbage and the overflow stage was never drained while the memory stage stayed live. Ref: vectordotdev#20322
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thank you for your contribution! Before we can merge this PR, please sign our Contributor License Agreement. To sign, copy and post the phrase below as a new comment on this PR.
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
There was a problem hiding this comment.
More details
Interrupted disk reads preserve the record length and accumulated payload, allowing subsequent polls to resume the same record without losing its boundary when memory-stage traffic wins.
🤖 Bits Code Review · Commit 2597d35
Summary
A memory buffer with
when_full: overflowinto a disk buffer never drains the disk stage while the memory stage keeps receiving events. Spilled events stay on disk until a restart, or are never delivered.BufferReceiver::nextpolls the disk reader insideselect!against the memory stage and drops the disk read future whenever the memory stage wins.RecordReader::try_next_recordis not cancel-safe: it reads the length delimiter, then fillsaligned_bufover severalfill_buf/consumecalls. When the future is dropped in between, the bytes already consumed are gone, and the next call reads the middle of a record as a length delimiter. The reader then fails on garbage or waits for data that never arrives.This PR keeps the length of the record being collected in
pending_record_len, so a cancelledtry_next_recordresumes the same record instead of starting over. The buffer is reset only when a new record starts, and the pending state is cleared on errors and once the record is complete.References
Closes: #20322
Related: #24304
Related: #26033 (touches the same reader; it does not change record-read cancellation, so the two should compose)
Vector configuration
How did you test this PR?
overflow_disk_stage_drains_while_memory_stage_is_liveintopology/channel/tests.rs: spills records to the disk stage, then keeps the memory stage live and asserts that every spilled record is delivered without recreating the buffer. It fails on master and passes with this change.cargo test -p vector-bufferspasses.known_errors::writer_detects_when_last_record_has_scrambled_archive_datais flaky with and without this change.systemctl restart vectorin the middle of the outage: the disk stage contents survived the restart and were delivered after unblocking.Does this PR include user facing changes?
no-changeloglabel to this PR.