Reset ingester shards periodically while running - #6769
Conversation
The startup advise_reset_shards only runs once: if it fails (e.g. the control plane is restarting at the same time) or shards become stale while the ingester keeps running, nothing deletes or truncates them until the next restart, and the WAL keeps growing (quickwit-oss#6531). Re-run the existing reset periodically from a background task holding a weak state reference, like the other ingester tasks. Fixes quickwit-oss#6531.
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. |
![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: b1e95acab6
ℹ️ 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".
| // A reset is already in progress. | ||
| continue; | ||
| }; | ||
| state.wait_for_ready().await; |
There was a problem hiding this comment.
When an ingester enters Retiring before a periodic retry, this call waits forever because decommissioning only advances the status to Decommissioning/Decommissioned, never back to Ready. This is especially harmful in the startup-failure scenario addressed here: an unreconciled stale shard can prevent check_decommissioning_status from completing, while the periodic task that could delete it is permanently stalled, causing graceful decommission to time out.
Useful? React with 👍 / 👎.
1 participant
Reset ingester shards periodically while running
Summary
Quickwit ingesters only reconcile their local shards with the control plane in two situations: at startup, and when the WAL disk usage crosses 90%. If the startup
advise_reset_shardsRPC fails, for example, because the control plane itself is restarting at the same time during a rolling restart, the ingester keeps its stale local shards (and keeps growing its WAL) until it is restarted again. Nothing retries the reset while the ingester keeps running.This adds a background task that re-runs the existing
reset_shards()reconciliation every minute, so stale local shards converge without a manual restart. It reuses the existing periodic-task shape (CloseIdleShardsTask) and the existingadvise_reset_shardsRPC; no protocol or control-plane change.Context
Fixes #6531.
As analyzed in the issue, the reconciliation itself works well when it runs (the reporter observed ~1k orphaned shards cleared in 5s), but it is only reachable at ingester startup or on the 90%-disk path, which with the default WAL limit (~3.6 GiB) never fires in many clusters. In the reported incidents, a single untouched ingester leaked WAL growth for 60 hours until an unrelated restart ran its startup reset.
Why this is safe to run periodically, unlike what the issue initially feared ("a reset could delete valid shards while the control plane is starting up"):
advise_reset_shardsis served by the control plane actor, which only processes messages afterinitialize()has finished loading the full model from the metastore. A periodic advise can therefore never be answered by a partially-loaded model; delete decisions always come from the complete shard table.What changed
quickwit-ingest/src/ingest_v2/ingester.rs: newspawn_periodic_reset_shards()background task spawned fromIngester::try_new, next to the existing startupbackground_reset_shards(). Following the shape of the other ingester background tasks (CloseIdleShardsTask), it holds a weak reference to the ingester state, skips the first interval tick (startup already ran a reset), and then re-runs the existing reset everyPERIODIC_RESET_SHARDS_INTERVAL(1 min in production, 1 s under test), stopping when the ingester is dropped. The reset logic itself was extracted into a freereset_shards(...)function shared by the startup path and the periodic task; the existingreset_shards_permitssemaphore still guarantees at most one reset at a time.test_ingester_periodic_reset_shards_recovers_from_startup_failure: an ingester recovers a shard from a pre-populated WAL (as after a restart), the startupadvise_reset_shardsfails withUnavailable, and the test asserts the periodic reset eventually deletes the orphaned queue without any restart. The test fails (times out) with the periodic spawn removed and passes with it.IngesterForTestgained awith_wal_dirbuilder so a test can restart the ingester over a pinned WAL directory.How was this PR tested?
cargo test -p quickwit-ingest test_ingester_periodic_reset_shards_recovers_from_startup_failure -- --nocapturecargo test -p quickwit-ingest test_ingester_reset_shards -- --nocapturecargo test -p quickwit-ingest --testscargo clippy -p quickwit-ingest --tests -- -D warnings