Repository navigation
Conversation
…nput When a reload changes a sink whose buffer is reused (or that has a disk buffer), the topology cancels the sink's detach trigger and then waits for the sink task to finish so that it can hand the buffer to the new sink. A sink that never polls its input again never finishes, so the reload hangs forever and later signals, including SIGTERM, are never handled. The TCP socket sink does this while it retries a connection that keeps failing. Once a sink's inputs are detached, give it up to the graceful shutdown limit to finish, then drop it and return its buffer anyway. Events the sink had already taken from the buffer are dropped. With no graceful shutdown limit, the old behavior of waiting forever is kept. Closes vectordotdev#26687 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 423ee0f380
ℹ️ 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".
Codex Review SummaryThis 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. |
The graceful shutdown limit comes from the command line, so a config loaded for a reload has no limit. Sinks built by a reload therefore had no limit on how long they get to finish once a later reload detached them, and a second reload of a stuck sink still hung. Carry the running topology's limit over to the new config, and extend the test to two reloads. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nk-26687 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
A config reload hangs forever if it changes a sink that has stopped polling its input. While it hangs, Vector ignores SIGTERM. A
socketsink does this while it retries a connection that keeps failing (#26687).Why it hangs:
RunningTopology::shutdown_diffcancels the sink's detach trigger, which ends its input stream viatake_until_if(tripwire). Then it awaits the old sink task with no timeout. The task's output (TaskOutput::Sink(rx)) is how the buffer gets handed to the new sink.TcpSink::runpeeks one event and then callsconnect_backoff(). That retries forever and never looks at the input again, so the task never finishes.StartedApplication::mainawaits the reload inside aselect!arm.The fix is in the topology, in
build_instrumented_sink. The sink task already ownsrxand only lends it tosink.run(rx.by_ref()...). So the task now racessink.run(...)against "inputs detached, then the graceful shutdown limit elapsed". If the timer wins:TaskOutput::Sink(rx)is returned as usual, so the reload completes and the new sink takes over the same buffer.Notes:
--graceful-shutdown-limit-secs(Config::graceful_shutdown_duration). With--no-graceful-shutdown-limit, the old wait-forever behavior is kept. A config loaded for a reload doesn't carry that value, soreload_config_and_respawnnow copies the running topology's limit into the new config. Without that, sinks built by a reload would have no limit.true, which happens whenshutdown_diffcancels the trigger. Removed inputs and normal shutdown disable the trigger instead.httpsink) looks like the same pattern, but I haven't verified that case.EventArray. Previously those events were stuck forever and the process needed SIGKILL.I didn't fix the inside of the TCP sink. With an event already peeked, the sink can't tell that its input has ended without consuming more events.
Not addressed here:
References
socketsink is reconnecting, and SIGTERM is then ignored #26687Vector configuration
From #26687; nothing listens on
127.0.0.1:7000:The reload adds
keepalive: {time_secs: 30}to the sink, then sends SIGHUP.How did you test this PR?
New test:
topology::test::topology_reload_replaces_sink_that_stops_polling_input.test_backpressuremock sink, which takes one event and then never polls again.basic_sink. It asserts that both reloads complete and the final sink receives later events.masterit fails (the first reload times out after 10 s). Without therunning.rschange, the second reload times out.Existing
topology::tests: all pass, exceptreload::topology_reload_preserves_enrichment_table_state. That test also fails on unmodifiedmasterwhen the wholetopology::suite runs in parallel on my machine, and it passes when run alone, with or without this change.Manual testing with a debug build on Linux x86_64,
--graceful-shutdown-limit-secs 10, and the config above:master, the reload never completes, and Vector is still running 20 s after SIGTERM.Also ran
cargo fmt --checkandcargo vdev check changelog-fragments. Clippy (-D warnings, with the features I built) reports nothing in the changed files.Does this PR include user facing changes?
no-changeloglabel to this PR.Contributor Guidelines
@vectordotdev/vectorto reach out to us regarding this PR.git merge origin masterandgit push.🤖 Generated with Claude Code