Fix/cluster activate stranded secondary node - #1210
Open
wmousa wants to merge 5 commits into
Open
Conversation
…ation get_secondary_nodes() and get_secondary_nodes_2() each pair nodes one at a time via a greedy walk, preferring a domain/host-disjoint candidate from a shrinking shared pool. Nothing guarantees that walk closes a single cycle spanning every online node: it can close a cycle over a strict subset and strand the rest with zero candidates, even though a perfect pairing exists whenever there are 2+ online nodes (hit live: 12 nodes across 3 failure domains formed an 11-node secondary-pairing cycle, stranding the 12th and aborting activation with "No enough secondary nodes"). The tertiary assignment used by max_fault_tolerance >= 2 clusters (e.g. 2+2) has the identical structure and is subject to the same failure mode. Add splice_stranded_secondary() and splice_stranded_tertiary(): when a node is left with no candidates, splice it into an already-formed pairing edge (P->X becomes P->stranded->X) instead of giving up, preferring an edge where both sides differ from the stranded node's failure domain. The tertiary splice additionally re-validates host-disjointness against each side's own secondary partner, since a tertiary must be host-disjoint from both a primary and that primary's secondary. _cluster_activate falls back to these before raising, and only still fails if no pairing has been made at all yet.
_check_ftt_allows_node_removal gates node shutdown/suspend purely on raw not-online node count (cap = npcs), independent of the operator's drain-gate. This is stricter than necessary with failure domains enabled: placement guarantees at most one erasure-coding chunk per domain once there are ndcs+npcs distinct domains, so losing up to npcs domains at once is already tolerated the same way losing up to npcs nodes is tolerated without FD � but this check still blocked a second node in the same domain, undermining the drain-coordinator's FD-aware concurrency (observed live: a same-domain concurrent drain was correctly waved through by the operator's gate, then independently rejected here with "FTT=1: cluster already has 1 not-online node(s)"). With FD enabled and the target node's domain assigned, the capacity check now counts distinct affected domains instead of raw node count: piling onto an already-affected domain is always free; a new domain is gated on distinct-domain count against npcs when there are enough domains for full one-chunk-per-domain isolation, or falls back to the plain node-count cap otherwise (mirrors the operator's fdDrainGate). FD disabled or an unassigned node falls back to the original node-count logic unchanged. The npcs=2/ft=1 primary-secondary pairing constraint is unaffected.
…ndependent get_secondary_nodes/get_secondary_nodes_2/splice_stranded_secondary/ splice_stranded_tertiary each fetch their own candidate list fresh from the DB and scan/score it in that order, independent of whatever order the caller processes primaries in. With failure domains enabled, this made the resulting primary/secondary/tertiary assignment sensitive to arbitrary node ordering: even when a fully domain-disjoint assignment exists (e.g. equal- sized domains), ~1 in 5 arbitrary orderings left some node with a same- domain secondary or tertiary (verified by simulation), and the live deployment hit exactly this. All four functions now sort their fetched node list by failure_domain before scanning, which makes equal-sized domains fully order-independent (0 conflicts across 50 arbitrary orderings, verified). _cluster_activate's own pairing loop also sorts its processing order the same way: once domain sizes are uneven and splice-repair is required, the repair works off whatever partial assignment already exists, so the caller's processing order still mattered even with the candidate-scan fix alone. With both in place, unequal-domain conflict counts become deterministic instead of order-dependent. Both sorts are no-ops when failure domains are disabled.
…y check The previous rule treated piling additional nodes onto an already-affected failure domain as always free, only falling back to a raw node-count cap when opening a brand-new domain under-provisioned. Cross-checked against the backend team's confirmed tolerance (2 FD: one whole FD down OR one node in each FD, nothing else; 3 FD: one whole FD only; 4 FD: two whole FDs), that rule incorrectly allowed unsafe combinations such as one node in FD1 plus two nodes in FD2 on a 2-FD cluster. Each domain's worst-case contribution to a stripe's chunk loss is now capped at chunks_per_domain = ceil((ndcs+npcs) / domains_available). A domain already at or above that count has maxed its risk contribution, so further nodes in the same domain are free; otherwise the summed capped risk across all affected domains plus the node being removed must stay within npcs. This collapses to the existing "npcs whole domains free" behavior once there are >= ndcs+npcs domains, and reproduces the confirmed 2/3/4-FD tolerance exactly -- verified both by direct simulation of the formula and by driving the real function through every scheme/domain-count combination in tests/unit/test_ftt_protection.py.
…ctivation The bare correctness minimum for the interleaved rotation layout is npcs+1 distinct domains (2 for npcs=1, 3 for npcs=2) -- below that even the initial static placement is wrong: at exactly 2 domains the tertiary role mathematically always lands back in the primary's own domain, since "2 steps ahead" in a period-2 round-robin wraps to where it started (verified directly against rotation_layout: 8/8 tertiary placements landed same-domain at 2 domains, 0/12 at 3+). But a minimum-correct STATIC layout has zero spare hosts per domain, and the moment a single node is added or removed, the relocation logic (_pick_replica_relocation_node) has no spare candidate left to reassign the stranded role to. Verified directly: removing one node from a bare-minimum npcs=1/2-domain or npcs=2/3-domain layout strands another node's secondary/tertiary with _pick_replica_relocation_node returning None -- blocking the removal outright, not just degrading placement quality. This also matches the backend team's confirmed stance that a 2-FD layout can never absorb a second independent failure once one domain is down, so it's excluded at any npcs level. Fresh activation now hard-requires npcs+2 distinct domains (3 for npcs=1, 4 for npcs=2) -- one domain of spare capacity beyond the bare correctness floor, so a later single add/remove has somewhere to place the relocated role. Extracted as fd_activation_domain_count_violation() in planner.py (alongside fd_balance_violation, same pattern) since _cluster_activate itself has no unit-test mocking infrastructure and was otherwise untestable; 7 new tests cover the boundary directly.
wmousa
force-pushed
the
fix/cluster-activate-stranded-secondary-node
branch
from
August 5, 2026 16:24
d40f6d4 to
bcdd4a2
Compare
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.
No description provided.