enhancement: add an explicit log line for graceful component shutdown - #25974
Conversation
github-actions
Bot
added
the
domain: topology
This comment has been minimized.
This comment has been minimized.
![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: 26f75090ee
ℹ️ 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".
| // Finishes once all tasks have shutdown. | ||
| let success = futures::future::join_all(wait_handles).map(|_| ()); | ||
| let success = futures::future::join_all(wait_handles).map(|_| { | ||
| info!("All components shut down gracefully."); |
There was a problem hiding this comment.
When shutdown is triggered by a component error or panic, the handles in wait_handles still resolve because their results are mapped to () before join_all, so this branch logs All components shut down gracefully. even though handle_errors has already reported a component abort. That makes the new log unreliable as a clean-shutdown marker in the graceful-crash path; only emit it after successful task results, or change the wording to indicate that all handles merely finished.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Here, the property that I care about is making sure that all buffers were drained before exiting. I’m not entirely sure what "clean-shutdown" means here exactly. Maybe the wording should explicitly mention buffer draining?
26f7509 to
90db551
Compare
The shutdown sequence logs are the same whether the components where gracefully shutdown or not. Checking for the absence of the "Failed to gracefully shut down in time. Killing components." can match both a clean shutdown, or an OOM or a sigkill. Explicitly logging a graceful shutdown lets us work around this.
90db551 to
d8a3d05
Compare
|
|
||
| if all_done { | ||
| info!("Shutdown reporter exiting: all components shut down."); | ||
| info!("All components shut down gracefully."); |
There was a problem hiding this comment.
the log line above says about the same - do we need another one?
There was a problem hiding this comment.
this allows to have a single message for graceful shutdown in both cases.
2 participants
Summary
Add an explicit log line when components shut down gracefully during the shutdown sequence.
The shutdown sequence logs are the same whether the components where gracefully shutdown or not. Checking for the absence of the "Failed to gracefully shut down in time. Killing components." can match both a clean shutdown, or an OOM or a sigkill. Explicitly logging a graceful shutdown lets us work around this.
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.