fix: Drop stale routing entries on NoShardsAvailable failures. Closes #6324#6566
fix: Drop stale routing entries on NoShardsAvailable failures. Closes #6324#6566osyniakov wants to merge 6 commits into
Conversation
When an index was deleted and recreated, the router's per-ingester routing entry for the old incarnation could stay marked as having open shards because the ingester's piggybacked routing update only covers sources it still holds. Persist retries then kept picking the dead entry and the request surfaced as a 503 until Chitchat eventually caught up. Treat a `NoShardsAvailable` failure as a signal that this (leader, index_uid, source_id) has no reachable shard and zero it out in the routing table. If no nodes remain for that (index_id, source_id) the next attempt re-queries the control plane, which returns the fresh incarnation's shards. Fixes quickwit-oss#6324
Clarifies the hidden contract the fix leans on: the zero-out and piggybacked routing update run under the same lock, which is what keeps the rate-limited subcase of NoShardsAvailable correct.
Address PR review: introduce RoutingTable::mark_node_no_shards instead of calling apply_capacity_update(.., 0, 0). The new method only zeros the open_shard_count and leaves the capacity_score untouched (capacity is a node-level WAL signal independent of any specific source). It also no-ops on missing entries/nodes and on incarnation mismatches, so a narrowing signal can never roll back a fresher entry.
…shards Address PR review: replace the != short-circuit with the same Less / Equal / Greater cmp match used by apply_capacity_update and merge_from_shards. A stale signal (entry newer than the failure's index_uid) is still ignored; a signal for a newer incarnation now advances the entry, drops stale nodes, and forces a CP re-seed — consistent with how the rest of the routing table handles monotonic incarnations.
|
@nadav-govari @guilload could you please review this one? |
![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: 6d5fbd0b8f
ℹ️ 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".
| if let Some(node) = entry.nodes.get_mut(node_id) { | ||
| node.open_shard_count = 0; |
There was a problem hiding this comment.
When a source has more than four stale leaders (for example an index with ingest_settings.min_shards >= 5 spread across ingesters), zeroing only the node that just returned NoShardsAvailable burns one ingest attempt per stale leader. retry_batch_persist stops after MAX_PERSIST_ATTEMPTS (5), so the router can exhaust all attempts before has_any_routing_candidate becomes false and the control plane is queried; recreated indexes in that configuration can still return NoShardsAvailable until gossip clears the old entries. Consider invalidating all nodes for the failed (index_uid, source_id) when the routing update does not refresh it.
Useful? React with 👍 / 👎.
2 participants
Description
When an index was deleted and recreated, the router's per-ingester routing entry for the old incarnation could stay marked as having open shards because the ingester's piggybacked routing update only covers sources it still holds. Persist retries then kept picking the dead entry and the request surfaced as a 503 until Chitchat eventually caught up.
Treat a
NoShardsAvailablefailure as a signal that this (leader, index_uid, source_id) has no reachable shard and zero it out in the routing table. If no nodes remain for that (index_id, source_id) the next attempt re-queries the control plane, which returns the fresh incarnation's shards.How was this PR tested?
Automated tests
Fixes #6324