fix(qwen3): proactive cancellation sweep for queued requests#646
fix(qwen3): proactive cancellation sweep for queued requests#646sparkzky wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f766656a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
c665d72 to
8860c12
Compare
|
Hi, thanks for your PR! |
|
Hi, thanks for the heads-up! Noted — I'll make sure to leave a comment on the corresponding issue first next time before opening a PR. |
|
Could you please add reproducible E2E evidence to the PR description, including the exact command and full output, ideally covering the real HTTP disconnect path? |
|
Hi @xiaguan, thanks for the review! Added a Rust HTTP integration test ( The test starts the actual
Reproduce with: OPENINFER_TEST_MODEL_PATH=/path/to/Qwen3-4B \
cargo test --release -p openinfer-qwen3 --test http_cancellation -- --nocapture |
250915e to
33de043
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33de043b81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8afdfba to
60533fb
Compare
…er-project#642) Cancelled requests in deferred/loading/prefilling/active queues were only retired reactively at token emission — by then the full prefill had already burned GPU. Under a disconnect storm, cancelled work consumed scheduler capacity for tens of seconds. Add sweep_cancelled_requests() at the top of each scheduler iteration (after drain, before admission) that checks token_tx.is_closed() across all queues and drops cancelled requests via executor.drop_request(). The 'loading' queue is excluded: drop_request blocks on prefetch DMA wait, which would stall the scheduler thread. Cancelled requests in 'loading' settle naturally via reclaim_ready_prefetch and are swept when they reach 'deferred' on the next iteration. publish_load() moved to after the sweep so load metrics reflect post-sweep reality before the scheduler parks idle. KV remove events are re-drained after the sweep drops requests so a KV-aware router sees freed blocks immediately. TOCTOU race fixed: retain is driven by collected IDs, not by re-checking is_closed() (Codex review). Tests: - 3 CPU-only scheduler tests (deferred, prefilling, active cancel) - GPU integration test (EngineHandle): burst + disconnect → 0/0 in 250ms - HTTP integration test (real server): 8 streaming clients → disconnect → 0/0 in 59ms, follow-up served Closes openinfer-project#642 Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>
60533fb to
4851070
Compare
Summary
Qwen3 scheduler only discovered cancellation reactively at token emission — by then the full prefill had already burned GPU. Under a disconnect storm, cancelled work consumed scheduler capacity for tens of seconds.
Closes #642.
Root cause
admit_deferred_requestsnever checkedtoken_tx.is_closed()— cancelled deferred requests were admitted and ran full prefill before the first token send failed.ContinuePrefill(effects.rs:316), which only covered mid-chunk cancellation.publish_load()ran before the sweep at the top of the loop, so after a sweep cleared everything the scheduler parked idle without republishing(0,0)— load metrics froze stale.Changes
1.
sweep_cancelled_requests()— proactive sweep (scheduler.rs)One function called at the top of each scheduler iteration (both
scheduler_loopandscheduler_loop_with_lora_control), after drainingsubmit_rxand before admission:token_tx.is_closed()across all queues:deferred,loading,prefilling,post_control_deferred,activeexecutor.drop_request()(KV block RAII release, prefetch DMA wait, saved_cursor cleanup)is_closed(), to avoid a TOCTOU race (Codex review)inflight_prefill_pendingis deliberately excluded — its KV is mid-write during decode-overlap prefill; documented in the function doc2.
publish_load()reordered (scheduler.rs)Moved to after
sweep_cancelled_requests()in both loops. Per-iteration order is now: drain → sweep → publish_load → prefetch/admit. This ensures the published metrics reflect post-sweep reality before the scheduler parks idle.Verification
cargo test -p openinfer-qwen3 --libcancellation_sweep(GPU, EngineHandle)http_cancellation(GPU, real HTTP path)scheduler_robustness(regression)New tests
CPU-only scheduler tests (
scheduler/tests.rs):sweep_drops_cancelled_deferred_request_before_prefill— cancelled deferred + post_control_deferred requests never reach prefill; live sibling survivessweep_drops_cancelled_prefilling_request_and_releases_state— cancelled mid-chunk request releases executor state; live sibling keeps its placesweep_drops_cancelled_active_request_from_running_set— cancelled decoding request leaves running set and returns KV; live sibling keeps decodingGPU integration test — EngineHandle (
tests/cancellation_sweep.rs):cancelled_burst_metrics_collapse_promptly— submits 10 concurrent requests, drops all receivers, assertsnum_running=0 && num_waiting=0within bounded time, then verifies a clean follow-up completion succeedsGPU integration test — real HTTP disconnect path (
tests/http_cancellation.rs):http_disconnect_metrics_collapse— starts the realopeninferserver, sends 8 concurrent streaming/v1/completionsrequests, disconnects all clients, polls/metricsfor collapse, then sends a clean follow-upE2E Evidence — real HTTP disconnect path
Exact command
OPENINFER_TEST_MODEL_PATH=/path/to/Qwen3-4B \ cargo test --release -p openinfer-qwen3 --test http_cancellation -- --nocaptureFull output
The
auto-aborting request due to dropped streamlog lines confirm the full bridge abort path: HTTP client disconnect → bridge detects dropped stream → setsabort_reason→ scheduler sweep callsdrop_request→/metricsreflects zero.Acceptance criteria
drop_requestat a safe scheduler boundary and leave the running set