fix: shut down query-frontend and querier gracefully - #7967
Open
electron0zero wants to merge 13 commits into
Open
electron0zero wants to merge 13 commits into
electron0zero wants to merge 13 commits into
Conversation
electron0zero
requested review from
carles-grafana,
ie-pham,
javiermolinar,
knylander-grafana,
mapno,
mattdurham,
mdisibio,
ruslan-mikhailov,
stoewer,
yvrhdn,
zalegrala and
zhxiaogg
as code owners
September 21, 2026 21:47
electron0zero
force-pushed
the
fix_incorrect_shutdown
branch
2 times, most recently
from
September 22, 2026 18:40
35876d8 to
dc0de07
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5ced048. Configure here.
3 tasks
dskit stops the cleanup ticker before calling stopping(), so the wait for an empty queue map could never be satisfied and SIGTERM hung until SIGKILL.
query-frontend and querier enable polling with context.Background(), so Reader.Shutdown() waited forever for a poll loop that never exits.
Without a cluster seed, running() returned context.Canceled on SIGTERM, so dskit marked usage-report failed on every clean shutdown.
A service that never terminates used to hang the process with nothing in the logs. This named the second deadlock on its first run.
Stop() returns nil once a service exits cleanly, which the shutdown fixes now make the normal case, so the unguarded deref would panic.
Guards the bug class rather than the two instances found: fails if any component is SIGKILLed on SIGTERM. Validated against the pre-fix binary.
The rebase onto the v3 module migration (grafana#7971) replayed these files without rewriting their imports, breaking lint and vendor check under -mod=vendor.
Refuse new work once shutdown starts so the drain converges, fail anything it could not dispatch, and answer a stopped queue with 503 instead of a 500.
…IGTERM At 1% the bound held only while the frontend refused for under 20ms.
traces_created_total can increment before the trace answers a query.
The 2s lookback raced a live-store cold start. Raise it to 20s and the gap to 25s.
electron0zero
force-pushed
the
fix_incorrect_shutdown
branch
from
September 23, 2026 19:35
3d1efd2 to
ea18bdc
Compare
Unrelated to shutdown. They move to their own PR.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does:
deadlock stopped query-frontend and querier from calling
server.Shutdown()on shutdown, so these services waited for the grace period and it was SIGKILLeddeadlocks happened because request queue waited on a cleanup ticker dskit had already stopped, and the store waited on a blocklist poller.
Also stops usage-report reporting itself as failed on a clean shutdown.
Added test and new e2e test to ensure all components have a clean shutdown and this class of bugs don't happen again.
we are not sure when we got this bug because from what I recall we have been seen dropped connections during rollouts for a while in the logs.
we reworked this queue logic in #3996 but I think this bug existed way before that and been around from early days of tempo.
Which issue(s) this PR fixes:
Relates to #7983
Checklist
.chloggen/(runmake chlog-new, ormake chlog-new FILENAME=<name>to override the default branch-name file; see.chloggen/README.md)