[Bugfix] Keep the multi-port DP supervisor serving through fault-tolerant recovery - #54963
Open
Etelis wants to merge 4 commits into
Open
[Bugfix] Keep the multi-port DP supervisor serving through fault-tolerant recovery#54963Etelis wants to merge 4 commits into
Etelis wants to merge 4 commits into
Conversation
…rant recovery A halted rank recovers under its own sentinel and a removed rank exits while its peers keep serving; tear down only when no rank is left and derive readiness from the sentinels. Signed-off-by: Itay Etelis <itay.etelis@ibm.com>
…e interval Per-rank /health stays 200 while the group is halted, so once fault tolerance is on and the group has been ready the supervisor polls /fault_tolerance/status instead. Signed-off-by: Itay Etelis <itay.etelis@ibm.com>
…ure threshold Set the shutdown event before tearing down the children, tear down only after dp_supervisor_probe_failure_threshold empty sentinel sweeps, and keep the stock rules for the Rust frontend. Signed-off-by: Itay Etelis <itay.etelis@ibm.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Etelis
marked this pull request as ready for review
September 3, 2026 07:10
Etelis
requested review from
AndreasKaratzas,
DarkLight1337,
NickLucche,
aarnphm and
robertgshaw2-redhat
as code owners
September 3, 2026 07:10
Contributor
Author
|
@njhill @tylertitsworth |
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
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.
With
--data-parallel-multi-port-external-lb --enable-fault-tolerance, a failed DP rank halts the group, the engines recover in place within a few seconds afterscale_down, and the removed rank's API server exits when its own sentinel gives up (engine_recovery_timeout_sec, 120s).DPSupervisortreats both as a group failure:_monitor_childrentears everything down as soon as any child pid exits, and_probe_all_childrendoes the same as soon as any child/healthis not 200. So the supervisor kills a pool that already recovered, ~120s after the fault:The same probe keeps
/readyat 200 while the whole group is halted, because a rank's/healthonly checks that its engine process is alive.Fix: once fault tolerance is on and the group has been ready, the supervisor polls each remaining child's
/fault_tolerance/statusinstead of/health. A rank reportingunhealthyis recovering; a child that exited while peers are alive was removed by the engine and its port is dropped from the probe set. The supervisor tears down only afterdp_supervisor_probe_failure_thresholdconsecutive sweeps in which no rank is serving or recovering, the same tolerance the/healthprobe had./readyreflects "at least one rank healthy";/healthbecomes a liveness signal that stays 200 until the supervisor itself decides to shut down, so a liveness probe on it does not race the recovery. Nothing changes without--enable-fault-tolerance.Depends on #46370 (fault tolerance in external-LB mode). Related: #41438 (in-engine supervisor for internal-LB), ai-dynamo/grove#793.
Test
E2E on H200: two pools of
DeepSeek-V2-Lite, each one pod with--data-parallel-size-local 4 --data-parallel-multi-port-external-lb --enable-expert-parallel --all2all-backend nixl_ep --enable-eplb --eplb-config '{"num_redundant_experts": 24}' --enable-fault-tolerance, #46370 applied, behind an istio Gateway + llm-d endpoint picker v0.10.0 (InferencePool targetPorts 8000-8003). Pod readiness on:9256/ready, liveness on:9256/health. Load: 16 clients, ~16 rps, unique prefixes, 300s; at t=60sSIGKILLofWorker_DP1_EP1in pool A. Same image and gateway for both columns.scale_downsent at +32s: pool A at +125sfound 1 exited DP Servers), 2/2 runsscale_downsent at +32s: failed requests after +120sscale_down: pod NotReady atscale_down: failed requestsSupervisor log with the fix:
Failures inside the halt window (requests already parked on the pool, or routed to the dead port until it exits) are the same with and without this PR; readiness is per pod while the picker's endpoints are per port.