Skip to content

[PD] Fix MORI-IO ABORT bootstrap message handling - #29133

Merged
HaiShaw merged 11 commits into
sgl-project:mainfrom
maning00:fix-bootstrap
Aug 28, 2026
Merged

HaiShaw merged 11 commits into
sgl-project:mainfrom
maning00:fix-bootstrap

Conversation

@maning00

@maning00 maning00 commented Jun 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix Mori decode-side ABORT notifications being treated as malformed bootstrap messages because MORI_GUARD frame validation.
This dispatches ABORT before guarded bootstrap payload validation, marks tracked rooms as failed, and avoids stale status/failure-record cleanup issues after abort.


CI States

Latest PR Test (Base): ✅ Run #32573036042
Latest PR Test (Extra): ❌ Run #32573035969
Latest PR Test (AMD ROCm 7.2): ❌ Run #32573036069

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@maning00 maning00 changed the title Fix Mori ABORT bootstrap message handling [PD] Fix MORI-IO ABORT bootstrap message handling Jun 24, 2026
@maning00
maning00 marked this pull request as ready for review June 26, 2026 06:14
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@billishyahao billishyahao added amd run-ci CI: run the baseline test suite on this PR labels Jul 15, 2026
@ShangmingCai

Copy link
Copy Markdown
Collaborator

/rerun-test test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Rebase Required Before Re-run

A major update has landed on main. Your PR is diverged relative to required base commit 3e096629cf98.

Re-run was not dispatched. What to do:

  • Rebase your branch onto the latest main and push again
  • Follow issue #21065 for context
  • CI-fix PRs may request the bypass-maintenance label to skip this check

@ShangmingCai

Copy link
Copy Markdown
Collaborator

/rerun-test test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py

@github-actions

github-actions Bot commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

Results for /rerun-test test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py:

⛔ test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py: No register_cuda_ci(runner_config=...) or register_cpu_ci() found in test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py. This file may not be a registered CI test.

@ShangmingCai

Copy link
Copy Markdown
Collaborator

CC: @HaiShaw

@maning00

Copy link
Copy Markdown
Contributor Author

The AMD ROCm 7.2 failure is pre-existing on main and unrelated to this PR: test_disaggregation_basic.py::TestDisaggregationSimulatedRetract::test_gsm8k aborts the decode scheduler with a GPU memory access fault inside UnifiedRadixCache.retraction_restore, and the same job fails with the identical signature on main at this PR's base commit 3c69a4c744 (https://github.com/sgl-project/sglang/actions/runs/32572153356/job/97029077762). That test runs with --disaggregation-transfer-backend mooncake, and get_kv_class() imports the mori module only in the TransferBackend.MORI branch, so mori/conn.py — the only file this PR touches — is never imported into the crashed process.

FWIW /rerun-test cannot re-run this suite either: detect_suite() in scripts/ci/utils/slash_command_handler.py only recognizes register_cuda_ci(runner_config=...) and register_cpu_ci(), while this file registers via register_amd_ci().

@Duyi-Wang

Copy link
Copy Markdown
Collaborator

@amd-bot ci-status

@amd-bot

amd-bot commented Aug 24, 2026

Copy link
Copy Markdown

@Duyi-Wang

CI Status for PR #29133

Merge verdict: ⚠️ Do not merge on a green signal. Two independent problems: (1) this PR's changed code is not exercised by any PR-CI test that actually ran, and (2) PR CI is incomplete — stage-b failures fast-fail-skipped 12 downstream AMD ROCm 7.2 jobs and 9 PR-Test-Extra jobs. Of the failures that did execute (AMD: 5, NPU: 1, CPU: 1), zero are related to this PR — they are pre-existing MoE / perf-threshold / attention / mooncake-disagg / infra failures in code paths this +55/−0 MORI-only change never touches.

Caution

The one test that covers the changed file — test/registered/amd/disaggregation/test_mori_transfer_engine_e2e.py (the only PR-CI test that runs the mori transfer backend) — never ran. It is registered in suite stage-b-test-large-8-gpu-mi35x-disaggregation-amd, but the runner executes test_disaggregation_basic.py first, that file failed (mooncake backend, unrelated), and the job aborted with exit 255 before reaching the MORI file. Green does NOT verify this PR. Moreover, even that e2e test is only a test_generate_smoke — it would not exercise the decode-side ABORT-during-bootstrap path this PR actually fixes. Before merge, run the MORI e2e suite in isolation on an MI35x runner, and ideally add a test that drives an ABORT frame through the bootstrap socket.

Changed files: python/sglang/srt/disaggregation/mori/conn.py (+55/−0)

Executed CI failure attribution: AMD: 5 failures (0 related) · Others: 2 real failures (0 related) + NPU perf-gate cascades. Downstream AMD/Extra stages skipped by fast-fail (not tested).

AMD Executed Failures

Job Test File Test Function Error Related? Why
mi35x-8gpu-disaggregation test/registered/amd/disaggregation/test_disaggregation_basic.py TestDisaggregationSimulatedRetract.test_gsm8k (0/4) RuntimeError: Internal Server Error / connect fail :11200 🟢 Runs mooncake backend in CI (is_in_ci() forces mooncake); PR only edits mori code path
1-gpu-small (7) test/registered/moe/test_fused_moe.py (6/9 passed) FAILED (failures=1) 🟢 Fused MoE kernel; no relation to PD disagg
1-gpu-small (0) test/registered/moe/test_torch_compile_moe.py TestTorchCompileMoe.setUpClass TypeError: ... not JSON serializable; server exit −9 🟢 torch.compile MoE; unrelated
1-gpu-large (1) test/registered/perf/test_bench_serving_1gpu_part2.py TestBenchServing1GPUPart2.test_score_api_latency_throughput AssertionError: 52.91 not less than 48 🟢 Perf threshold on reranker; unrelated
mi35x-1gpu-small test/registered/attention/test_verify_shared_kv.py TestVerifySharedKV.test_rejects_multiple_local_kv_heads AssertionError: True is not false 🟢 Attention KV-head check; unrelated

Other Executed Failures

Job Test File Test Function Error Related? Why
multimodal-gen-1-npu-a3 sglang/multimodal_gen/test/server/ascend/test_server_1_npu.py TestDiffusionServerOneNpu::test_diffusion_generation[*] Perf validation asserts (e.g. 3347.5 <= 443.0) + container error 🟢 NPU (Ascend) diffusion perf; NPU has no MORI path
build-test (xeon-gnr, base-b-test-cpu) test/registered/cpu/test_subblock_sparse_attention.py N/A ModuleNotFoundError: No module named 'imageio' + HF gated-repo 401 🟢 Env/infra (missing dep + gated model auth); unrelated

(The four base-c-test-perf-{2,4,8,16}-npu-a3 "failures" are health-check fast-fail cascades of the NPU multimodal-gen failure above, not independent test failures. The *-finish, wait-for-stage-b-amd-rocm720, and call-gate / pr-gate jobs are aggregators/gates reflecting the same failures.)

CI completeness gaps (fast-fail skipped — NOT tested)

  • AMD ROCm 7.2 (run 32573036069): wait-for-stage-b-amd-rocm720 failed → 12 downstream jobs skipped, incl. all stage-c-* (4-gpu, large-8-gpu, dsv4-fp4/fp8), sgl-kernel-unit-test, jit-kernel-*, multimodal-gen-*, and call-pr-test-amd-extra-rocm720.
  • PR Test Extra (run 32573035969): CPU build-test failed → 9 downstream skipped (extra-a/b GPU tests, rust-ext-build, sgl-kernel-build-wheels).
  • PR Test (Base) is ✅; NPU skipped only multimodal-gen-test-2-npu-a3.

What to do before merge

  1. Verify the change actually runs. Run test_mori_transfer_engine_e2e.py in isolation on an MI35x/gfx950 runner (mori backend). It is currently blocked behind the unrelated mooncake test_disaggregation_basic.py failure in the same suite.
  2. Add ABORT-path coverage. The fix targets decode-side ABORT bootstrap frames; the existing smoke e2e test won't hit _handle_abort_message. Consider a unit test that pushes a [b"ABORT", room] multipart through bootstrap_worker.
  3. The 5 AMD + 2 Other executed failures are pre-existing and unrelated — they should be triaged by CI owners, not this PR author, but they currently mask/block the signal for this PR (they abort the disagg suite and fast-fail the downstream stages). Re-running won't help unless those pre-existing failures are fixed or the MORI test is unblocked; the bypass-fastfail label would let downstream AMD stages run but still won't reorder the disagg suite so the MORI file executes.

Generated by amd-bot using Claude Code CLI

@HaiShaw HaiShaw 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.

MoRI specific

@HaiShaw
HaiShaw merged commit 9fc60a8 into sgl-project:main Aug 28, 2026
157 of 175 checks passed
jambow0320 added a commit to jambow0320/sglang that referenced this pull request Aug 30, 2026
Resolved two conflicts introduced by upstream:

* mori/conn.py: sgl-project#29133 landed an equivalent Mori ABORT handler
  (_TAG_ABORT / _handle_abort_message) that holds transfer_lock across the
  status read-modify-write and returns early on an already-Failed room.
  Kept upstream's handler and dropped this branch's _handle_abort_notification,
  which duplicated it without the lock. sgl-project#29133 also covers the Mori
  _submit_kv_transfer cleared-room guard this branch carried.

* nixl/conn.py: kept the cleared-room guard's room_transfer_infos local
  alongside the packed_source_by_dcp_rank map added by sgl-project#35762; the two
  changes are adjacent but independent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

amd run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants