Remove departed ingesters from routing table - #6680
Conversation
![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
https://github.com/quickwit-oss/quickwit/blob/40c141e2b7a95154a370228fcef1836a186ff051/quickwit-serve/src/lib.rs#L1260-L1262
In clusters where an indexer only transitions out of ready state, this now publishes IngesterDeparture and the router deletes all routing entries for that node. ClusterChange::Remove is emitted for a ready→not-ready transition as well as for an actual leave (quickwit-cluster/src/change.rs:239-246), and when the node becomes ready again setup_ingester_pool just re-inserts it into the pool without restoring those routing entries. That creates a transient ingest regression for sources whose shards were still valid on that same ingester: requests have to go back through control-plane/capacity refresh instead of routing immediately. Consider distinguishing real departures/rejoins (for example with the chitchat generation or an explicit event kind) from readiness flaps before clearing the routing table.
ℹ️ 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: 37268e24ef
ℹ️ 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 mut num_entries = 0; | ||
|
|
||
| for entry in self.table.values_mut() { | ||
| if entry.nodes.remove(node_id).is_some() { |
There was a problem hiding this comment.
When a ClusterChange::Remove races with an in-flight control-plane response or a delayed capacity-score event from the same ingester, this removal is not durable: neither merge_from_shards nor apply_capacity_update records that the node has departed, so a stale update can reinsert the node immediately after this line. In the restart/rejoin case, the old shard counts/capacity become eligible again as soon as the same node_id is back in the pool, leaving the stale-routing window this change is meant to close; consider tracking a departure generation/tombstone or rejecting updates for nodes known to have left.
Useful? React with 👍 / 👎.
| return; | ||
| }; | ||
| let mut state_guard = state.lock().await; | ||
| let num_entries = state_guard.routing_table.remove_node(&departed_ingester.node_id); |
There was a problem hiding this comment.
When the departed ingester is the only known leader for a source, deleting it here also deletes the only evidence the next GetOrCreateOpenShardsRequest uses to populate unavailable_leaders. If this router observes the removal before the control plane's ingester pool does, the follow-up CP request is sent with an empty unavailable list, so the CP can return the same stale open shard for the removed node_id and the router re-seeds the route instead of forcing a replacement shard; keeping a tombstone/zero-capacity entry long enough to report the leader unavailable would avoid that propagation race.
Useful? React with 👍 / 👎.
|
On hold as we discuss a more thorough, more complicated fix |
2 participants
Description
Ingesters that leave the cluster aren't removed from the routing table. We were relying on the ingester pool to not select them. If these ingesters later rejoin the cluster, the old entry is still there- representing stale data. It's only cleared when the control plane inits shards on those ingesters, or they broadcast their first capacity update (both can take seconds). This clears the entry on departure to prevent that edge case.
How was this PR tested?
Unit tests.