[controllers] fix: surface cuda mps runtime failures - #133
Conversation
|
Warning Review limit reached
More reviews will be available in 50 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesCUDA/MPS Runtime Health Status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the propagation of fatal post-start CUDA and MPS worker failures to the service status via the allocation_status() hook. It introduces a _failure_exc attribute to track unexpected exceptions (excluding recoverable out-of-memory errors) and ensures that release() correctly cleans up resources for dead, failed workers. Extensive regression and unit tests have been added to verify these behaviors across both CUDA and MPS controllers. I have no further feedback to provide as there are no review comments to evaluate.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/macm_controller/test_macm_backoff.py (1)
104-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing MPS invalid-config regression test.
MacMGPUController._keep_loop()now reportsinvalid vram_to_keep=...throughallocation_status()when_num_elements <= 0, but this suite only covers worker/allocation failures and OOM retries. CUDA already has the parity test, so an MPS regression on that branch would currently slip through.Suggested test shape
+def test_macm_records_invalid_post_start_num_elements_as_failure(): + ctrl = MacMGPUController.__new__(MacMGPUController) + ctrl.rank = 0 + ctrl.device = "mps" + ctrl.interval = 0.01 + ctrl.busy_threshold = -1 + ctrl.iterations = 1 + ctrl.vram_to_keep = 0 + ctrl._num_elements = 0 + ctrl._failure_exc = None + ctrl._stop_evt = _StopWaitForbidden() + + ctrl._keep_loop() + + error = ctrl.allocation_status() + assert isinstance(error, RuntimeError) + assert str(error) == "rank 0: invalid vram_to_keep=0"As per path instructions, "Behavior changes in single-GPU controllers must be kept in sync with related docs and tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/macm_controller/test_macm_backoff.py` around lines 104 - 290, Add a regression test in the MacM backoff suite for the invalid configuration path in MacMGPUController._keep_loop(). Create a controller with _num_elements set to 0 or below, run the loop, and assert allocation_status() returns a RuntimeError whose message matches the new invalid vram_to_keep=... failure text. Use the existing MacMGPUController setup pattern in the other tests to keep it consistent with the MPS worker/allocation/OOM coverage.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/macm_controller/test_macm_backoff.py`:
- Around line 104-290: Add a regression test in the MacM backoff suite for the
invalid configuration path in MacMGPUController._keep_loop(). Create a
controller with _num_elements set to 0 or below, run the loop, and assert
allocation_status() returns a RuntimeError whose message matches the new invalid
vram_to_keep=... failure text. Use the existing MacMGPUController setup pattern
in the other tests to keep it consistent with the MPS worker/allocation/OOM
coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ddac72e-5ab6-45fe-bf03-67245d00892c
📒 Files selected for processing (8)
AGENTS.mddocs/plans/cuda-mps-runtime-health-status.mdsrc/keep_gpu/single_gpu_controller/cuda_gpu_controller.pysrc/keep_gpu/single_gpu_controller/macm_gpu_controller.pytests/cuda_controller/test_throttle.pytests/global_controller/global_keep_test.pytests/macm_controller/test_macm_backoff.pytests/single_gpu_controller/test_release_contract.py
|
Resolved CodeRabbit's MPS invalid-config coverage nitpick in follow-up commit Follow-up verification after the change:
|
1 participant
Summary
Verification
13 passed)58 passed, 10 skipped)558 passed, 11 skipped)Local Review
Summary by CodeRabbit
New Features
Bug Fixes