Repository navigation
Conversation
a6a2bc4 to
b7df047
Compare
fd18659 to
7b9f6e0
Compare
graphcareful
left a comment
There was a problem hiding this comment.
I like the overall idea but I think the implementation itself can be simplified
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2163401395
ℹ️ 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".
bruceg
left a comment
There was a problem hiding this comment.
Two optional follow-ups below: the removal warn currently goes silent in the case I think we care about most, and stop() still does not wait on the handles we now keep. I would not hold merge for either.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3301482025
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3648a88cd
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf02daa2c6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5619bb4c95
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78a4ddfce4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 061e707aac
ℹ️ 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".
| pub(super) struct OrphanedDiskBufferScanner { | ||
| sender: watch::Sender<Option<ScanRequest>>, | ||
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fce10c15dd
ℹ️ 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".
| } else { | ||
| "Removing disk-buffered sink with remaining accounted data; reload will wait while its current instance finishes processing." | ||
| }; | ||
| warn!( |
There was a problem hiding this comment.
nit: Should this one be a warning? Maybe some of these new messages can be replaced by info! instead as no data is being lost nor does it require human intervention
There was a problem hiding this comment.
These new warnings can be accompanied by new metrics too and we could move some of this code into new internal events.
Once this is done we can improve the changelog to explain what "provides lifecycle visibility" means with those metrics
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 798fc425a6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf3c163958
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43cc0595c8
ℹ️ 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".
List each lifecycle counter event in the changelog with when it fires and what it means. Explain that unavailable buffer usage means a disk buffer writer was busy. Remove a semicolon from the unreferenced buffer warning and pass the usage snapshot through directly when emitting the replacement event.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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 usage = self | ||
| .inputs | ||
| .get(*key) | ||
| .expect("changed sink input must exist") | ||
| .disk_usage_snapshot(); |
There was a problem hiding this comment.
Sample the disk buffer that owns the reported path
When a disk-backed ordinary sink shares a runtime key with a memory enrichment-table sink, full startup builds the table sink last (src/topology/builder.rs:629-649), so self.inputs[key] is the table's memory sender while sink_buffer(&self.config, key) above selects the ordinary disk config. If the ordinary sink is then changed to memory, this samples Some(0) from the memory sender and emits a replacement_unreferenced warning claiming no bytes were observed even when the reported old disk directory has persisted backlog; the post-reload scan also temporarily excludes that previous path (src/topology/running.rs:354-356). Keep active-buffer ownership metadata (or sample the selected disk buffer itself) so the usage belongs to buffer_dirs.
Useful? React with 👍 / 👎.
Summary
data_dirthat are not referenced by the active configuration.Graceful shutdown behavior is intentionally unchanged here; that behavior is isolated in #26238.
Vector configuration
No standalone configuration is required. The diagnostics apply to sinks configured with disk buffers during startup and configuration reload.
How did you test this PR?
vector-bufferslibrary suite: 115 passed, 2 ignored, covering disk-buffer encoding, filtering, overflow, acknowledgement, recovery, and usage accounting-D warningscargo fmt --all -- --checkIs this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Related: #26238