fix(workers): preserve Marker GPU allocation from Ray resources - #452
fix(workers): preserve Marker GPU allocation from Ray resources#452hedhoud wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough_marker_num_gpus now returns 0 for non-positive requests, checks Ray's cluster_resources for "GPU" when configured >0, and falls back to torch.cuda.is_available() on Ray errors; tests added to validate the Ray-path; a dispatcher test had AsyncMock formatting adjusted. ChangesMarker GPU Allocation Robustness
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openrag/services/workers/parsers/marker_workers.py (1)
27-34: ⚡ Quick winConsider adding observability to the exception fallback path.
When
ray.cluster_resources()raises an exception and the function falls back to the CUDA check, there's no log indicating this occurred. Adding a warning log would help operators understand GPU allocation decisions and diagnose issues when Ray cluster resources are unavailable.📊 Suggested addition for observability
def _marker_num_gpus(config) -> float: requested_gpus = config.loader.marker_num_gpus if requested_gpus <= 0: return 0 try: return requested_gpus if ray.cluster_resources().get("GPU", 0) > 0 else 0 except Exception: + logger.warning( + "Failed to query Ray cluster resources for GPU detection; falling back to CUDA check" + ) return requested_gpus if torch.cuda.is_available() else 0🤖 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 `@openrag/services/workers/parsers/marker_workers.py` around lines 27 - 34, In _marker_num_gpus, add a warning log in the except block so operators know ray.cluster_resources() failed and the code is falling back to torch.cuda.is_available(); capture and include the exception details in the log (e.g., use logging.getLogger(__name__).warning or the module's existing logger) and then proceed to return the existing fallback value based on torch.cuda.is_available() and config.loader.marker_num_gpus; reference symbols: _marker_num_gpus, config.loader.marker_num_gpus, ray.cluster_resources(), torch.cuda.is_available().Source: Coding guidelines
🤖 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 `@openrag/services/workers/parsers/marker_workers.py`:
- Around line 27-34: In _marker_num_gpus, add a warning log in the except block
so operators know ray.cluster_resources() failed and the code is falling back to
torch.cuda.is_available(); capture and include the exception details in the log
(e.g., use logging.getLogger(__name__).warning or the module's existing logger)
and then proceed to return the existing fallback value based on
torch.cuda.is_available() and config.loader.marker_num_gpus; reference symbols:
_marker_num_gpus, config.loader.marker_num_gpus, ray.cluster_resources(),
torch.cuda.is_available().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 913f814d-55e6-440d-9a11-e6b2517d5015
📒 Files selected for processing (2)
openrag/services/workers/parsers/marker_workers.pytests/unit/services/workers/parsers/test_marker_workers.py
b95d4f8 to
4f263b2
Compare
Context: Phase 11 made Marker PDF parsing much slower because GPU allocation can be decided from a Ray actor that does not itself own GPU resources.\n\nThis keeps Marker worker GPU requests based on Ray cluster resources, while still falling back safely when GPU resources are absent. That prevents PDF serialization from silently becoming CPU-bound on GPU deployments.\n\nCloses #451.
Summary by CodeRabbit