fix(buffers): Stop reporting usage for dropped buffers - #25994
Conversation
The buffer usage reporter looped forever, so it outlived the buffer it reported on. After a configuration reload replaced a sink's buffer, the reporter for the old buffer kept publishing gauges under the same `buffer_id` as its replacement, and both the task and the stage state it held were retained for the life of the process. The reporter now discards each stage once it holds the only remaining reference to it, after one final report to pick up the deltas recorded since the previous tick, and the task ends once no stages remain. That also lets the metrics for a buffer which was removed rather than replaced expire. Since the reporter's lifetime now depends on the buffer holding on to its usage handle, that requirement is documented on `IntoBuffer::provides_instrumentation`.
bruceg
added
domain: observability
![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: 8d45a67be2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stage.report(current_metrics, buffer_id); | ||
|
|
||
| // Every handle for the stage has been dropped once the reporter holds the only remaining | ||
| // reference, and no new handle can appear from a count of one. | ||
| Arc::strong_count(stage) > 1 |
There was a problem hiding this comment.
When the last BufferUsageHandle is active concurrently with this tick, stage.report can consume a category before that handle records its final update; if the handle then updates the counter and drops before strong_count is checked, this returns false and permanently discards the unreported delta. Capture whether another reference exists before calling report, so a stage that was live at the start of the tick is retained for one more, race-free final report.
Useful? React with 👍 / 👎.
1 participant
Summary
The buffer usage reporter looped forever, so it outlived the buffer it reported on. After a configuration reload replaced a sink's buffer, the reporter for the old buffer kept publishing gauges under the same
buffer_idas its replacement, and both the task and the stage state it held were retained for the life of the process.The reporter now discards each stage once it holds the only remaining reference to it, after one final report to pick up the deltas recorded since the previous tick, and the task ends once no stages remain. That also lets the metrics for a buffer which was removed rather than replaced expire.
Since the reporter's lifetime now depends on the buffer holding on to its usage handle, that requirement is documented on
IntoBuffer::provides_instrumentation.Vector configuration
How did you test this PR?
See unit tests. This was uncovered while expanding the buffers' benchmark set.
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details on the dd-rust-license-tool.