Skip to content

sglang 0.9.18, vllm 0.8.17: pd-ds drift liveness also detects in-place engine restart - #72

Merged
remotego merged 2 commits into
mainfrom
pd-ds-router-engine-identity
Oct 20, 2026
Merged

remotego merged 2 commits into
mainfrom
pd-ds-router-engine-identity

Conversation

@remotego

Copy link
Copy Markdown
Collaborator

Summary

The pd-ds router's drift probe from 1f0d236 keys on DNS growth: a new upstream IP appearing in the headless Service restarts the router. That misses in-place engine restart — same IP, new process.

On vllm + mooncake that case is real and silent. vllm_pd_router.rs:2100 queries each prefill's /query once at boot and pins the returned engine_id into every subsequent kv_transfer_params. When the kubelet restarts a crashed prefill in place, the pod keeps its IP, DNS shows no growth, and the router keeps addressing a dead engine_id — transfers hang/fail indefinitely, no restart.

This PR extends the drift probe to also catch the in-place swap, using the new GET /engine endpoint from hang-watcher#5. Both charts (sglang, vllm) patched; NIXL unaffected (its handshake is lazy per-transfer).

How it works

Boot script (pd-ds.yaml router container): after the existing DNS-resolve loop, also GET http://<ip>:9090/engine for each upstream and append <ip>|<restarts>|<watcher_started_at> per line to the existing /run/pd-router/<role>-ips snapshot (sorted, atomic mktemp+mv, no new files).

Liveness probe (_pod.tpl pdRouterLivenessProbeScript): per tick, re-query /engine on each argv'd IP and compare against the boot tuple. Fires iff:

  • both sides returned real tuples and they differ → in-place engine swap, or
  • boot recorded sentinel but live returns real → late-arriving identity (restart so the router re-snapshots with full info)

and the /health gate from 1f0d236 passes (still: don't restart into a still-loading engine).

Compatibility

  • hangWatcher.enabled: false: HW_PORT=0 rendered into probe and boot script short-circuits every query to the sentinel. One-line switch, single code path.
  • Old hang-watcher (no /engine): 404 → sentinel; same as sidecar-disabled render.
  • Boot transient (503/conn-refused/timeout): retry up to 90 min, then sentinel + proceed (fail-open, mirrors the DNS budget).

Rendered output

8 helm-verify cases diff, all in the pd DisaggregatedSet router's Deployment (pd-ds.yaml args) and liveness probe (_pod.tpl). 46 cases unchanged. Baked gold per R3 (./regen-gold.sh HEAD), re-verified 54/54 green.

Test plan

  • helm template renders clean for every pd case, both charts
  • bash -n on all 4 rendered scripts (vllm boot + liveness, sglang boot + liveness)
  • ast.parse on the embedded python heredoc
  • E2E against mock hang-watchers: same-id / 503 / sentinel→real / real→different-real / 404 paths
  • Live test on test-dsset cluster (reviewer): restart a vllm-mooncake prefill in place, watch the router liveness fire within one probe interval

🤖 Generated with Claude Code

…e engine restart

1f0d236 taught the pd-ds router's drift probe to fire on engine /health
so the router restarts the moment a newly-scheduled engine is actually
usable, and no earlier. That probe keys on *IP set growth*. It misses
the case where an engine's IP is unchanged but the process behind it
was replaced.

That case is real on vllm + mooncake: the router pulls each prefill's
engine_id off its /query endpoint once at boot (vllm_pd_router.rs) and
pins it into every kv_transfer_params. When the kubelet restarts a
crashed engine in place, the pod keeps its IP, DNS shows no growth,
but the engine's new process has a fresh engine_id -- the router
keeps addressing a dead incarnation and transfers hang or fail.
NIXL dodges this because its handshake is lazy per-transfer.

The fix piggybacks on the hang-watcher sidecar that already runs on
every engine pod. With modelsphere/hang-watcher#5 the sidecar serves
GET /engine on its existing healthz port, returning
{"restarts", "watcher_started_at", ...} where the pair is a stable
identity of the engine process the sidecar is watching. No new RBAC,
no router-version dependency, no engine-side changes required.

Boot script (pd-ds.yaml): after DNS resolves the upstream IPs, also
query each engine's /engine endpoint and record
"<ip>|<restarts>|<watcher_started_at>" per line in the existing
/run/pd-router/<role>-ips file. 404 on /engine = old sidecar without
the endpoint, record "<ip>||" sentinel. 503 / conn refused / timeout
= transient: retry up to 90min, then fall through to the same
sentinel and proceed (fail-open, mirrors the DNS budget). The sentinel
makes the runtime probe's answer "no identity info", identical to
the hang-watcher-disabled render.

Liveness probe (_pod.tpl): per tick, re-query /engine on each argv'd
IP. Fires iff:
  - both sides returned a real tuple and they differ  (in-place swap)
  - boot recorded sentinel but live returns real      (late-arriving
    identity; restart so the router re-snapshots with identity)
and the usual /health gate passes (mirrors 1f0d236: don't restart
into a still-loading engine).

hangWatcher.enabled=false renders HW_PORT=0 into the probe and boot
script, short-circuiting every query to the sentinel: one-line switch,
single code path.

Old-sidecar compat: /engine 404s, boot records sentinel, liveness
never has a live real tuple to compare against -> drift probe
degrades to pre-patch behavior (IP growth only), still useful.

Chart versions: sglang 0.9.17 -> 0.9.18, vllm 0.8.16 -> 0.8.17.
…ngine

The pd router drift-liveness probe landed in the previous commit relies
on GET /engine, which is only served by hang-watcher >= 0.1.27. An older
tag silently degrades the probe to its pre-identity behavior (IP-growth
only): every query 404s, every row in /run/pd-router/<role>-ips records
the sentinel, and drift liveness never fires on in-place engine restart.

Pin 0.1.27 as the default tag in both charts' values.yaml, and extend
the comment block to carry the same "silent version skew is worse than
a hard error" rationale the cluster already uses for stallSec and
warmupSec. Operators overriding hangWatcher.image.tag to a mirror that's
stale now have a chance to read why their protection is gone.
@remotego
remotego merged commit 4ba1dc0 into main Oct 20, 2026
4 checks passed
@remotego
remotego deleted the pd-ds-router-engine-identity branch October 20, 2026 00:55
remotego added a commit that referenced this pull request Oct 20, 2026
The prior commit's subject still references 0.9.18 / 0.8.17, but those
versions belong to #72 (the engine-identity drift probe). This bump
re-attaches the log-lines change to its own version pair so the followup
release can be tagged cleanly.

No template / values change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant