Skip to content

[sgl-router] Fail fast and cancel decode when prefill fails - #41612

Merged
ShangmingCai merged 10 commits into
mainfrom
codex/pd-c-prefill-failfast
Oct 1, 2026
Merged

ShangmingCai merged 10 commits into
mainfrom
codex/pd-c-prefill-failfast

Conversation

@sherlockwu

@sherlockwu sherlockwu commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

A failed prefill cannot supply KV to decode, but the router previously ignored the prefill failure and left the client waiting for decode's bootstrap timeout.

Modifications

  • Fail-fast: decode's response goes to the client as soon as decode answers; its headers and stream do not wait for prefill. If prefill fails first, its failure is returned and decode is dropped, which aborts it on the engine via the router-minted request ID. If decode fails first, its error is returned and prefill keeps running. Each worker records its own outcome.
  • Status mapping: prefill 4xx, 429 and 503 are forwarded verbatim; router errors pass through (breaker-open 503, 504 timeout). Only a prefill fault (other 5xx, a panicked task, or a 200 stream carrying an SSE error event) becomes 502 prefill_failed.
  • Abort scope: a prefill failure after decode's headers aborts decode's stream only before its first token, since that token means the KV transfer finished; it is counted as stream_outcome_total{outcome="aborted"}. Prefill is never aborted. Caller-supplied IDs and sample fan-out keep their existing opt-out from prefix-based abort.

CI States

Latest PR Test (Base): ✅ Run #36851779946
Latest PR Test (Extra): ❌ Run #36851779649
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.

@sherlockwu
sherlockwu added this pull request to stack #41614 September 29, 2026 01:49
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 29, 2026
@sherlockwu
sherlockwu force-pushed the codex/pd-c-prefill-failfast branch from 3fa6702 to 3c5813d Compare September 29, 2026 04:32
@sherlockwu
sherlockwu force-pushed the codex/pd-c-prefill-failfast branch from 3c5813d to abdf910 Compare September 29, 2026 05:00
@sherlockwu
sherlockwu force-pushed the codex/pd-c-prefill-failfast branch from abdf910 to 73149d7 Compare September 29, 2026 05:37
@sherlockwu sherlockwu added router run-ci CI: run the baseline test suite on this PR and removed documentation Improvements or additions to documentation labels Sep 29, 2026
@sherlockwu
sherlockwu force-pushed the codex/pd-c-prefill-failfast branch from 9263d71 to 4c17013 Compare October 1, 2026 02:11
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 1, 2026
@ShangmingCai
ShangmingCai force-pushed the codex/pd-c-prefill-failfast branch from 4c17013 to 82af055 Compare October 1, 2026 06:32
Base automatically changed from codex/pd-b-optional-tokenizer to main October 1, 2026 10:30
sherlockwu and others added 9 commits October 1, 2026 18:30
Extract fail-fast forwarding from #40606 (8a7a482). Reuse the abort-on-drop implementation on main with safe router-generated PD request IDs; leave prefill detached. Exercise reorg failures before decode headers and after its SSE pump starts.
…il-fast

Charge a prefill 4xx/5xx/transport failure to the prefill worker's
metrics instead of the cancelled decode, log the prefill URL for every
failure, and drop the helper enum from forward_pd. Tests share one PD
context helper and replace the closed-port trick with a truncated body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A decode 429/5xx arrives as a response, not an error, so try_join! kept
waiting on a prefill whose KV transfer can no longer complete, then replaced
the decode status with 502. Treat any unsuccessful decode response as final,
and name the PD outcome instead of nesting results in a tuple.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A router-initiated abort ends the stream as `aborted`: it counts as neither a
client disconnect nor a breaker success, and leaves the engine abort armed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Decode's response is returned as soon as decode answers. A prefill failure
before then is returned instead; one after aborts decode's stream until its
first token. Router errors from prefill pass through unchanged, and prefill
client errors and backpressure are forwarded verbatim; only a prefill fault
becomes prefill_failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prefill task records its outcome on success and failure. The dispatch
record stays on decode, except that a failure blamed on prefill is logged
against prefill without charging decode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A streaming prefill reports a failure after its 200 as a data: {"error"...}
event, which previously counted as success.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g test setup

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ShangmingCai
ShangmingCai force-pushed the codex/pd-c-prefill-failfast branch from 82af055 to 161da2f Compare October 1, 2026 10:30
When prefill is blamed, `record_dispatch_result` skipped
`record_worker_request` entirely to avoid double-counting prefill, whose
task books its own outcome. That also dropped decode, which had already
reached the wire: `prefill_failure_aborts_decode_before_its_first_token`
waits on decode's captured body before releasing the failure, and the
request is then aborted on the engine by rid. So a dispatch that happened
left no trace in `sgl_router_worker_requests_total`.

That contradicts the metric's documented contract -- per-worker dispatches,
"recorded after dispatch; blind to pre-dispatch drops" -- for a *post*-dispatch
drop, and nothing else covers it: `stream_outcome_total` counts committed 2xx
streams only, and before decode's headers there is no stream. Decode's dispatch
counts therefore shrank during exactly the prefill incident an operator reads
them to understand.

Record the abandoned dispatch as `cancelled`, the outcome that already means
"the router ended this itself, not the worker's fault". Carry the decision in
`Blame` so it stays keyed to whether decode's future was ever polled: if prefill
fails before that, nothing reached the worker and the drop stays uncounted, as
the contract requires.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ShangmingCai
ShangmingCai merged commit 3b2ad1c into main Oct 1, 2026
95 of 100 checks passed
@ShangmingCai
ShangmingCai deleted the codex/pd-c-prefill-failfast branch October 1, 2026 11:35
ShangmingCai added a commit that referenced this pull request Oct 1, 2026
Resolves ten conflicts left by #41610, #41611 and #41612 landing on main while
this branch still carried their pre-squash commits. Two needed judgement rather
than a side:

- `readyz`: main gained the peer-bootstrap seed gate (#40699) while this branch
  rewrote the pool check for version groups. The hunks overlap, and taking
  either side whole drops the other, so the resolution keeps this branch's
  group-aware `pool_ready` *and* main's `&& ctx.kv_bootstrap_admit_ready()`.
- `chat/forward.rs`: this branch carries #41612's pre-fix copy; main has the
  post-fix one. The version-group feature does not touch this file at all, so
  it takes main's version, keeping the `cancelled` outcome #41612 records for a
  decode dispatch that prefill's failure abandoned.
- `pd_bootstrap_injection/reliability.rs` (add/add): keeps this branch's
  `..Default::default()` spec and main's two #41612 assertions.

The rest are the mechanical `..Default::default()` churn this branch introduced
meeting fields main spells out, plus a `readiness_rejects_portless_prefill` body
git aligned against its own merged copy; resolved so each test and helper is
defined once.

Verified on the merge result: 1017 lib + 129 component + 157 proxy tests pass,
clippy -D warnings and rustfmt clean. The two fixes a careless resolution would
have reverted are covered by tests that still pass -- the five
`pd_bootstrap_injection::reliability` cases and the four seed-gate `readyz`
cases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation router run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants