[PR #708] Make the host-side dispatch timeout self-attributing #722

New issue

Open

opened 2026-10-27 21:05:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-27 21:05:37 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/DeepEP/pull/708
Author: @dmvevents
Created: 7/29/2026
Status: 🔄 Open

Base: main ← Head: hostside-timeout-attribution

📝 Commits (3)

cf75cd6 Make the host-side dispatch timeout self-attributing
8b06138 Snapshot the counters once before formatting the timeout message
8574ff9 Deduplicate the timeout report and capture the throw line exactly

📊 Changes

1 file changed (+77 additions, -6 deletions)

View changed files

📝 csrc/legacy/buffer.hpp (+77 -6)

📄 Description

Problem

The two host-side CPU-wait timeouts in csrc/legacy/buffer.hpp report the symptom but not its origin.

buffer.hpp:596 (intranode dispatch) throws DeepEP error: CPU recv timeout with no diagnostics at all — no rank, no counters, no elapsed time.

buffer.hpp:1105 (internode dispatch) prints the total and per-expert counters, but the exception text itself carries nothing. Once the printf output is separated from the exception — multi-rank log interleaving, or only the Python traceback captured — the attribution is gone. The single surviving signal is -1, which says nothing about which counter never arrived, how long the wait actually was, or where the throw came from.

Change

Diagnostics only — no behaviour change. Both sites now carry, in the exception text:

resolved source location via __FILE__ / __LINE__
waited vs limit seconds
rank and num_local_experts
exactly which counters were still negative (total / rdma / each per-expert index), or an explicit note that all counters were ready at throw time — which would indicate a race rather than a stalled transport

The existing printf lines at the internode site are preserved verbatim, and both error strings keep their original prefixes (DeepEP error: CPU recv timeout, DeepEP error: timeout (dispatch CPU)) with new detail appended after.

Verification

Inserted logic extracted verbatim and compiled standalone, g++ -std=c++17 -Wall -Wextra — clean, no warnings.
Ran with a representative stall state (total=-1, rdma=-1, experts {-1, 0, -1, 5}); the message names experts 0 and 2 as stalled and correctly omits the two that were ready.
Log-scanner regression: the legacy Global rank: … line, all per-expert lines, and both error-string prefixes still match their original patterns; pre-patch logs from a real failure also still match the same scanner. A silently blinded scanner would turn a real timeout into a false pass, so this was verified rather than assumed.
Brace/paren balance re-checked across the file (197/197, 1378/1378).
Added #include <string>, which was not previously included directly, rather than relying on a transitive include for std::string/std::to_string.

Motivation

On AWS EFA we hit this exact timeout across a batch of 384-expert runs. The cause turned out to be a build-configuration regression in our own image — but the log could not have told us that; recovering it took source archaeology plus a bisect against a known-good image. With this patch the exception alone distinguishes "no counter ever arrived" (dead transport) from "some experts arrived and others did not" (partial routing), which is the first branch point in any such investigation.

Relation to #704

Complementary, non-overlapping. #704 makes the device-side low-latency trap() self-attributing (the origin of a sticky CUDA 719) in csrc/kernels/legacy/internode_ll.cu. This PR does the host-side twin in csrc/legacy/buffer.hpp, where the CPU busy-wait throws. Different files, different failure paths — in our own logs a word-boundary search for 719 / LAUNCH_FAILED / cudaErrorLaunchFailure returns zero, so our failures are purely the host-side path this PR covers. Both are needed for the same reason: today the error signal does not identify where it came from.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/DeepEP/pull/708 **Author:** [@dmvevents](https://github.com/dmvevents) **Created:** 7/29/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `hostside-timeout-attribution` --- ### 📝 Commits (3) - [`cf75cd6`](https://github.com/deepseek-ai/DeepEP/commit/cf75cd6a0637d8edd1e00e261f205c8c0d738043) Make the host-side dispatch timeout self-attributing - [`8b06138`](https://github.com/deepseek-ai/DeepEP/commit/8b06138ed31f1e9873fdc05e120722e78fc61e97) Snapshot the counters once before formatting the timeout message - [`8574ff9`](https://github.com/deepseek-ai/DeepEP/commit/8574ff9092e765823cde68cab0d5b3bacb1b561e) Deduplicate the timeout report and capture the throw line exactly ### 📊 Changes **1 file changed** (+77 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `csrc/legacy/buffer.hpp` (+77 -6) </details> ### 📄 Description ### Problem The two host-side CPU-wait timeouts in `csrc/legacy/buffer.hpp` report the symptom but not its origin. `buffer.hpp:596` (intranode dispatch) throws `DeepEP error: CPU recv timeout` with **no diagnostics at all** — no rank, no counters, no elapsed time. `buffer.hpp:1105` (internode dispatch) prints the total and per-expert counters, but the **exception text itself carries nothing**. Once the `printf` output is separated from the exception — multi-rank log interleaving, or only the Python traceback captured — the attribution is gone. The single surviving signal is `-1`, which says nothing about *which* counter never arrived, how long the wait actually was, or where the throw came from. ### Change **Diagnostics only — no behaviour change.** Both sites now carry, in the exception text: - resolved source location via `__FILE__` / `__LINE__` - waited vs limit seconds - `rank` and `num_local_experts` - **exactly which counters were still negative** (total / rdma / each per-expert index), or an explicit note that all counters were ready at throw time — which would indicate a race rather than a stalled transport The existing `printf` lines at the internode site are preserved **verbatim**, and both error strings keep their original prefixes (`DeepEP error: CPU recv timeout`, `DeepEP error: timeout (dispatch CPU)`) with new detail appended after. ### Verification - Inserted logic extracted verbatim and compiled standalone, `g++ -std=c++17 -Wall -Wextra` — clean, no warnings. - Ran with a representative stall state (total=-1, rdma=-1, experts `{-1, 0, -1, 5}`); the message names experts **0 and 2** as stalled and correctly omits the two that were ready. - **Log-scanner regression:** the legacy `Global rank: …` line, all per-expert lines, and both error-string prefixes still match their original patterns; pre-patch logs from a real failure also still match the same scanner. A silently blinded scanner would turn a real timeout into a false pass, so this was verified rather than assumed. - Brace/paren balance re-checked across the file (197/197, 1378/1378). - Added `#include <string>`, which was not previously included directly, rather than relying on a transitive include for `std::string`/`std::to_string`. ### Motivation On AWS EFA we hit this exact timeout across a batch of 384-expert runs. The cause turned out to be a build-configuration regression in our own image — but **the log could not have told us that**; recovering it took source archaeology plus a bisect against a known-good image. With this patch the exception alone distinguishes "no counter ever arrived" (dead transport) from "some experts arrived and others did not" (partial routing), which is the first branch point in any such investigation. ### Relation to #704 Complementary, non-overlapping. #704 makes the **device-side** low-latency `trap()` self-attributing (the origin of a sticky CUDA 719) in `csrc/kernels/legacy/internode_ll.cu`. This PR does the **host-side** twin in `csrc/legacy/buffer.hpp`, where the CPU busy-wait throws. Different files, different failure paths — in our own logs a word-boundary search for `719` / `LAUNCH_FAILED` / `cudaErrorLaunchFailure` returns **zero**, so our failures are purely the host-side path this PR covers. Both are needed for the same reason: today the error signal does not identify where it came from. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-27 21:05:37 +00:00

bsakaguchi referenced this issue

2026-10-27 21:05:42 +00:00

[PR #723] Fix legacy normal kernels trapping when hidden exceeds the TMA staging buffer #731