Skip to content

Fail only the request whose forward raised - #981

Merged
vasilevklart merged 2 commits into
mainfrom
users/kirill/per-rid-isolation
Nov 1, 2026
Merged

vasilevklart merged 2 commits into
mainfrom
users/kirill/per-rid-isolation

Conversation

@vasilevklart

@vasilevklart vasilevklart commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

On main, a raise in one request's forward inside Engine._exec_per_request, which runs unbatchable walks, fails every request in flight on the worker.

Now only the raising rid fails; _exec_single, for one-request or captured steps, still fails the batch. The slot fence and commit_done run on every exit, postprocess skips a failed rid so its first error is the one reported, and a failed rid keeps its pages until its REMOVE, since its kernels may still write them. Under TP/SP the ranks stay in step only if every rank raises on the same rid; if only some do, it breaks as on main.

1×H100, BAGEL (bagel_single_gpu.yaml), a hook raises after one request's forward; 120 s at c=8 and c=16 with cancels main this PR
healthy requests failed by another request's fault 45 (in 11 of 13 batches) 0
KV free after a quiesce fully fully

Measured on 6e78192ea (main 237832f8a) before the rebase; the clean batch's images are bit-identical across arms.

Tests: 6 cases in test_admit_failure_handling.py and test_admit_rollback.py fail on main and pass here; with test_worker_admit_lifecycle.py, 53 pass on 4276d852e. On c313af056 (main f83fcaeb3), CPU Core fails 18 here and 17 on main, the difference being TP read-failure sims that time out under that load and the eval runner's SIGTERM test, each passing alone 3 of 3 on both; TP lockstep sim 11/11, ruff clean.

Not run: a CPU-overhead harness; the batched path doesn't change, and the per-request path only adds a try.

Split out of #275 (threads 2, 7, 11); independent of #980.

AI assistance (Claude) was used for this PR.

@Gaurav-Shah05 Gaurav-Shah05 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I checked the TP case against main. A forward that raises on every rank fails the same row on all and thus keeps the ranks in lockstep. But, if it's only raised on only some ranks, then it puts them out of step on main too.

@Gaurav-Shah05 Gaurav-Shah05 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meant to approve

On the unbatched path each request runs its own plan, forward and commit, but a
raise in one of them left the loop and failed the whole batch, and with it
everything else in flight on the worker. The loop now records the error on that
rid, which the worker drops and fails as it does a prepare_inputs failure, and
runs the rest. The slot fence is recorded on every exit of a rid, so a later rid
on the same slot still waits out a raised one's queued copy, and the loop's own
way out always drains next_slot before it releases commit_done. Postprocess
skips a rid whose forward raised, so the client gets the forward's error.

A raised rid keeps its admitted pages and in-flight mark until the removal its
failure leads to, as a batch that raises does: unwinding them on the GPU thread
would free pages the forward's queued kernels may still write.
No behaviour change. Drops the two-rank gloo test (both ranks run the
same fake, so it can't catch an asymmetric failure), the removal test
that repeats the real-resource test's tail, and the forward case the
real-resource test covers; folds the every-rid-raises case into the
parametrized one.
@vasilevklart
vasilevklart force-pushed the users/kirill/per-rid-isolation branch from 4276d85 to 8512db1 Compare November 1, 2026 22:54
@vasilevklart
vasilevklart merged commit 41c77c9 into main Nov 1, 2026
6 checks passed
@vasilevklart
vasilevklart deleted the users/kirill/per-rid-isolation branch November 1, 2026 23:10
dghosef added a commit that referenced this pull request Nov 1, 2026
Main brings #981 (per-rid forward failures), #985, #367 and #916 (chunked
prefill as a graph Loop), #988 (Qwen3.5 on Blackwell), #977, #857 (stop
policies checked in the graph runtime), #852, #1076, #943, #1077 and #938.

Resolutions:
- Piecewise leases: main's #977 design. A forward replays a
  lease_before_step region exactly when its label is in
  ModelInputsFromEngine.piecewise_leases; #859's engine-side filter
  (SubmoduleManagement.runners_for) and its tests go. The forwards #859
  had moved onto runner presence check the lease again: Omnilingual's
  encoder loop and OmniVoice's backbone (both new here) as main's do, and
  V-JEPA2 and the FastConformer stream take main's code as it is.
- Qwen3.5: main's token_stop (runtime_check) beside #859's DeviceLoopback;
  main's _splice_row_ids replaces both loop-back paths in preprocess; decode
  builds its rotary tables in the graph always (#859), text prefill and mixed
  steps under ingraph_decode_rope (#988), whose knob entry, docstring and docs
  now say so; main's compile_deterministic and speculative_input_seq_len.
- TokenStop carries #859's withhold and #857's runtime_check.
- Orpheus: #859's TokenStop with runtime_check, so the runtime derives its
  EOS policy (the loop cap is the budget, as its check_stop has it), in place
  of #857's hand-written AR policy; test_stop_policies passes its TokenStop.
- Engine: chunking imports beside cudnn_benchmark.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants