Notice a ray worker that died without being stopped - #2622
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
f81e487 to
4032567
Compare
7f53e37 to
0acc726
Compare
a640571 to
da3ded2
Compare
6882f3c to
7f83aa9
Compare
da3ded2 to
8e607d4
Compare
The manager reported a cell alive as long as it still held actor handles, so a ServeActor that exited on its own — its rpc server stopped, its process was OOM-killed — stayed in the membership forever: the provider saw no change, and start_cells skipped the cell because its handles were not None. Nothing outside could repair it either, since only an explicit stop cleared them. External fault tolerance does not cover this: by default it watches rollout engines, which are command actors, and the static serve actors have no healing loop at all. Probing belongs to the actor, which owns the handle, so the death probe the worker handles already share becomes part of their contract. The loop belongs to the cell, which is the unit that gets torn down, and which keeps one slow probe from delaying every other cell's death. Each loop carries the generation it was started for and ends by itself once that generation is gone. Only a proven death counts: a probe that times out leaves the cell alone, so a worker busy in a long call is never mistaken for a dead one. Losing one worker tears down its whole cell, so the survivors are reclaimed, the membership says not alive, and start_cells can build the cell again.
8e607d4 to
d71e7f9
Compare
fzyzcjy
left a comment
fzyzcjy
left a comment
There was a problem hiding this comment.
The newly introduced membership lock is held over unbounded remote startup waits, preventing explicit or automatic teardown from recovering a stalled resume. The same blocker remains in deliver-2.
| logger.error(f"Starting cells {[c.cell_id for c in cells]} failed, rolling back", exc_info=True) | ||
| await asyncio.gather(*[c.stop() for c in cells], return_exceptions=True) | ||
| raise | ||
| async with self._membership_lock: |
There was a problem hiding this comment.
[P1] Let stop_cells interrupt a stalled start
When a resumed actor remains pending scheduling or its constructor/RPC startup never answers, start_cells holds this manager-wide lock across the unbounded alloc_ports and post_setup awaits. Both stop_cells and the liveness teardown require the same lock, so they cannot kill the pending actor—or another cell to free resources—and one failed resume wedges every membership operation indefinitely; make startup bounded/cancellable or avoid holding the lock while waiting on workers.
1 participant
Part of #1837