fix: fail fast in RayProcess with TaskGroup - #291
toby-coleman wants to merge 1 commit into
Conversation
|
Benchmark comparison for |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5297315423
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| coros = [component.step.remote() for component in self._component_actors.values()] | ||
| try: | ||
| await gather_except(*coros) | ||
| await _gather(*coros) |
There was a problem hiding this comment.
Mark cancelled Ray components as stopped
When one component call fails, _gather only cancels the sibling ObjectRefs; a component sets its status to RUNNING before step() or run(), and Ray cancellation exits without invoking Component.cancel() or another terminal status transition. Consequently, the cancelled actors and RayStateBackend can remain permanently RUNNING after the process becomes FAILED—the modified integration test now skips the previous STOPPED assertion for exactly this path. Trigger best-effort status cleanup for the cancelled component actors while preserving fail-fast propagation.
AGENTS.md reference: AGENTS.md:L26-L28
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| - LocalProcess | ||
| - RayProcess | ||
|
|
||
| `RayProcess` runs concurrent lifecycle calls in an `asyncio.TaskGroup`. When a |
There was a problem hiding this comment.
Remove this from the docs
There was a problem hiding this comment.
🟢 Approval recommended
The implementation is consistent with the stated behavior and includes targeted regression coverage.
Pull request overview
Updates RayProcess to fail fast and cooperatively cancel outstanding Ray calls.
Changes:
- Introduces TaskGroup-based ordered gathering and cancellation.
- Bounds attribute refreshes during failure handling.
- Adds unit/integration coverage and cancellation documentation.
File summaries
| File | Description |
|---|---|
plugboard/process/ray_process.py |
Implements fail-fast task coordination. |
tests/unit/test_ray_process.py |
Tests ordering, failures, and cancellation. |
tests/integration/test_ray_process_taskgroup.py |
Verifies cancellation with Ray actors. |
tests/integration/test_process_with_components_run.py |
Updates failure expectations. |
docs/api/process/process.md |
Documents cancellation behavior. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
A failing component could leave
RayProcesswaiting for every remote call, including blocked downstream components. Useasyncio.TaskGroupto propagate failures early and explicitly request cancellation of remote calls.Changes
gather_exceptthroughoutRayProcessand replace the blockingray.getused to update cancellation status with an async wait.