Skip to content

[Fix] Bind ring drain workers to their CUDA device - #110

Merged
XbzOnGit merged 3 commits into
mainfrom
fix/issue-105-drain-worker-cuda-device-binding
Aug 25, 2026
Merged

[Fix] Bind ring drain workers to their CUDA device#110
XbzOnGit merged 3 commits into
mainfrom
fix/issue-105-drain-worker-cuda-device-binding

Conversation

@XbzOnGit

Copy link
Copy Markdown
Collaborator

Closes #105.

Summary

Each native ring and D2H stream is created on the model worker's current CUDA device, but the background drain thread previously started with logical device 0 current. Under tight GPU-memory pressure, this caused the entire rank-1 capture to fail bit-for-bit validation—all 35,840 rank-1 rows failed—while model inference itself still completed.

This change records the owning device before creating the drain stream and selects that device at worker entry before any CUDA operation. A startup handshake makes cudaSetDevice() failure visible synchronously to the caller.

It also promotes failures from D2H enqueue, drain-stream synchronization, and the worker's final device synchronization from debug-only output to release-build diagnostics. This is diagnostic hardening only; transactional D2H failure handling is outside this PR.

Regression coverage

The new two-GPU native regression blocks work on device 0, constructs the ring and drain on device 1, and verifies that stopping the drain does not wait for device 0. This deterministically checks the worker's device binding without depending on GPU-memory pressure.

Validation

  • Native ring suite: 52 passed, 0 failed.
  • Qwen3-0.6B, BF16 eager, TP=2, vllm-full, vLLM GPU-memory utilization 0.96, 512 MiB payload and pinned-staging rings:
    • rank 0: 87,296 / 87,296 exact rows passed;
    • rank 1: 35,840 / 35,840 exact rows passed;
    • total: 123,136 / 123,136 exact rows passed.

@XbzOnGit
XbzOnGit requested review from Samfisheryu and zaoxing and removed request for zaoxing August 25, 2026 05:33
@zaoxing
zaoxing requested a balanced review from Copilot August 25, 2026 14:25

This comment was marked as off-topic.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

tests/native/ring/test_ring_engine.cu:285

  • The wait loop for callback_entered also busy-spins for up to 5 seconds. Adding a small sleep reduces CPU load in CI while still polling frequently enough for this test.
    while (!callback.entered.load(std::memory_order_acquire) &&
           std::chrono::steady_clock::now() < callback_deadline) {
        std::this_thread::yield();
    }

Comment thread native/csrc/ring/drain_thread.cpp
Comment thread native/csrc/ring/drain_thread.cpp
Comment thread tests/native/ring/test_ring_engine.cu
@XbzOnGit
XbzOnGit requested review from zaoxing and a lite review from Copilot August 25, 2026 15:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@zaoxing

zaoxing commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

LGTM

@XbzOnGit
XbzOnGit merged commit 9ba0e8a into main Aug 25, 2026
2 checks passed
@XbzOnGit
XbzOnGit deleted the fix/issue-105-drain-worker-cuda-device-binding branch August 25, 2026 15:38
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.

[Bug] Ring drain workers do not bind their owning CUDA device

3 participants