Skip to content

refactor(qwen35): split TP response contract into tp_executor/responses.rs - #1036

Open
CAICAIIs wants to merge 4 commits into
pegainfer-project:mainfrom
CAICAIIs:feat/qwen35-tp-responses-split
Open

refactor(qwen35): split TP response contract into tp_executor/responses.rs#1036
CAICAIIs wants to merge 4 commits into
pegainfer-project:mainfrom
CAICAIIs:feat/qwen35-tp-responses-split

Conversation

@CAICAIIs

@CAICAIIs CAICAIIs commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

What

Split the TP response contract out of tp_executor.rs into tp_executor/responses.rs, following #worker-split. Behavior unchanged.

Stacked PR 2/3 — based on #1035 (worker split); open against main because stacked cross-fork bases are not supported, so its Files-changed view includes #1035 until that merges. Merge #1035 first. Rebuilt over #1004/#1005/#1033/#1038: the precapture sweep keeps validate_ack_responses on the production path, so it stays visible rather than test-gated.

Structure

responses.rs (394 lines) owns the reply envelope (TpWorkerReply/TpWorkerResponse), bounded response collection with the 300s step timeout, the poison-on-failure contract, and per-command response validation. tp_executor.rs keeps the executor orchestration, commands, plan validators, and tests that don't exercise the response contract. The three response-contract tests (runtime_response_failure_poisons_executor, runtime_response_collection_fails_fast_when_peer_never_responds, disconnected_runtime_response_poisons_executor) move with the contract they pin.

Behavior-preserving consolidations (functional style)

  • validate_exact_rank_responses: the push loop folds into an into_iter().map(..).collect::<Result<Vec<_>>>() pipeline over the same checks (count, rank range, duplicate ranks, completeness), same error strings.
  • The three rank-0 payload validators (validate_prefill/decode/unified_responses) collapse into one validate_primary_responses helper over payload extractors, keeping the exact prior error messages (operation + payload noun cover all three wording variants).

Defense table (all inherited — pure move)

Defense Failure mode Successor
300s step timeout → abort a rank never answers, scheduler hangs forever inherited: responses.rs recv_runtime_response
Fail-fast collection on first failed rank peers blocked in a collective keep waiting inherited: responses.rs collect_runtime_responses
Channel-disconnect → poison (not silent Ok) replies lost after worker death inherited: responses.rs recv_runtime_response
Exact-rank set validation (count/range/dup/completeness) misrouted or replayed replies inherited: responses.rs validate_exact_rank_responses
Rank-0 primary payload + non-primary ack contract secondary rank returning artifacts inherited: responses.rs validate_primary_responses
Poison preserves first failure reason root cause overwritten by cascades inherited, pinned by the moved tests

Validation

  • cargo fmt --check clean; cargo clippy -p pegainfer-qwen35 --features qwen35 --all-targets -- -D warnings clean.
  • cargo test --lib green (moved response tests included); tp2_* GPU tests pass on 2×A100; e2e_scheduler green.

@CAICAIIs
CAICAIIs force-pushed the feat/qwen35-tp-responses-split branch 2 times, most recently from 5de248a to 8039d67 Compare September 8, 2026 06:52
@CAICAIIs
CAICAIIs force-pushed the feat/qwen35-tp-responses-split branch 2 times, most recently from 80b544a to 7b788fc Compare September 8, 2026 09:23
Move the per-rank worker runtime out of the tp_executor.rs God module:
TpWorker + spawn/drop lifecycle, TpStartupGate, the NCCL startup watchdog,
TpWorkerState/TpWorkerPrepared and its command loop (run/respond/
execute_*/precapture/drop), the slot bookkeeping helpers, the shared
decode-row view/sampling helpers, recurrent-capacity math, and the worker
CublasThreadGuard binding. The entry keeps command/reply types, the
executor orchestration (including the precapture sweep), plan validators,
and response validation, reaching worker items via the same `use super::*`
style as scheduler/{backend,tp}. Rebased over pegainfer-project#720/pegainfer-project#722. Behavior
unchanged; the watchdog disarm test moves into worker.rs with its
implementation.

Signed-off-by: CAICAIIs <3360776475@qq.com>
@CAICAIIs
CAICAIIs force-pushed the feat/qwen35-tp-responses-split branch from 7b788fc to 363facb Compare September 10, 2026 03:43
The tp_executor entry and the worker module carried two near-identical
condvar gates: TpCommandStartGate (Pending/Execute/Cancel, first-write
resolve returning whether it won) and TpStartupGate (Pending/Connect/
Cancel, silent set). Both broadcast one go/cancel decision that is
resolved exactly once, so they collapse into TpGate + TpGateDecision
with the first-write-wins resolve; startup treats Go as connect and
asserts on the returned decision instead of a bool. Behavior unchanged.

Signed-off-by: CAICAIIs <3360776475@qq.com>
The scheduler and the TP worker carried byte-identical CUDA/cuBLAS thread
bindings (guard + set_device + bind_to_thread + cublas_init); the worker
split moved one of the two copies, making the duplication visible. Both
now call crate::cublas_thread::bind_model_thread with a role tag (the
failure messages are unchanged); the scheduler keeps its
tune_decode_gemm_algos step at its own call site. Behavior unchanged.

Signed-off-by: CAICAIIs <3360776475@qq.com>
…es.rs

Move the reply envelope (TpWorkerReply/TpWorkerResponse), bounded response
collection, per-command response validation, and the step-timeout/abort
helpers out of tp_executor.rs; the three response-contract tests move with
them. Behavior unchanged, with two behavior-preserving consolidations in
the moved code: validate_exact_rank_responses folds its push loop into an
iterator pipeline, and the three rank-0 payload validators (prefill /
decode / unified) collapse into one validate_primary_responses helper over
payload extractors, keeping the exact prior error messages. Rebased over
pegainfer-project#1004/pegainfer-project#1005/pegainfer-project#1033/pegainfer-project#1038 (the precapture sweep keeps using
validate_ack_responses, which stays production-visible).

Signed-off-by: CAICAIIs <3360776475@qq.com>
@CAICAIIs
CAICAIIs force-pushed the feat/qwen35-tp-responses-split branch from 363facb to 676daed Compare September 10, 2026 05:19
@FeathBow

Copy link
Copy Markdown
Collaborator

@codex check this please

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

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.

2 participants