(5/7) feat(session): process-lifecycle layer — supervisor spawns N session workers + router#1565
(5/7) feat(session): process-lifecycle layer — supervisor spawns N session workers + router#1565guapisolo wants to merge 3 commits into
Conversation
guapisolo
requested review from
fzyzcjy,
jybsuper,
maocheng23 and
yueming-yuan
as code owners
![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 introduces a multi-process session server supervisor (SessionServerSupervisor) that manages the lifecycle of N session workers and 1 router process, ensuring fail-fast behavior on child process deaths. It also consolidates parent-death signaling (set_pdeathsig) and adds end-to-end tests. The review feedback highlights several critical robustness improvements for the supervisor, including: preventing process leaks during startup failures by tracking processes immediately, resolving a race condition in the monitor loop during shutdown, serializing concurrent shutdown calls with a lock, supporting IPv6 addresses in health checks, and avoiding memory leaks by unregistering the atexit handler upon shutdown.
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.
guapisolo
force-pushed
the
refactor/session-mp-lifecycle
branch
from
996fd19 to
c2af32a
Compare
guapisolo
force-pushed
the
refactor/session-mp-dataplane
branch
from
3421e64 to
801c443
Compare
guapisolo
changed the title
guapisolo
force-pushed
the
refactor/session-mp-dataplane
branch
from
801c443 to
d964716
Compare
guapisolo
force-pushed
the
refactor/session-mp-lifecycle
branch
from
c2af32a to
0fd7c91
Compare
…workers + router
SessionServerSupervisor runs in the rollout process and owns the child
processes of the multi-process session server: start() spawns ("spawn"
context — a forked child would inherit this Ray actor's threads and
finalizers, e.g. wandb's service thread, and deadlock) N workers plus one
thin router over socketpairs, closes the parent's socket ends so a child
death is observable as EOF, and gates readiness on the router reporting
every worker healthy. A monitor thread records the first child death and
tears the whole group down (SIGTERM, grace, SIGKILL — no orphans);
check() re-raises that failure so the rollout path can fail loudly
instead of hanging on a dead hash shard.
run_router is the router process entry point; it imports nothing from
core/worker, so the router process never loads the
tokenizer/transformers stack. set_pdeathsig moves to ipc
(stdlib-only) for the same reason, and the worker's private copy becomes
the shared helper.
test_session_supervisor spawns real worker+router processes, drives
create/chat/get end-to-end over IPC, then kills a worker and asserts
check() raises and no child is orphaned.
Nothing calls the supervisor yet; launch wiring lands separately.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
guapisolo
force-pushed
the
refactor/session-mp-lifecycle
branch
from
0fd7c91 to
d7d0a7d
Compare
…hutdown races - start(): register each child in _procs immediately after start() and widen the teardown try to the whole spawn phase, so a mid-spawn failure reaps every already-started child instead of orphaning them - _monitor_loop(): re-check _shutdown_done after observing a dead child; since shutdown() flips the flag before terminate(), a death seen with the flag still False is provably a real failure, never a false positive from normal teardown - shutdown(): serialize under a lock so a concurrent caller blocks until the group is fully reaped instead of returning while termination is in flight Addresses Gemini review comments 1-3 on #1565. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sweep the RST-style ``double backticks`` out of the session-server docstrings and comments — the repo convention is markdown-style single backticks, and the doubled form reads as a typo in review. Comment-only change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Shi-Dong
left a comment
Shi-Dong
left a comment
There was a problem hiding this comment.
Seems that the session supervisor only catches the case where a child process dies. If it hangs, nothing is triggered.
Not sure whether this is a lethal issue, though. LGTM now to deblock.
2 participants
Summary
The process-lifecycle layer of the multi-process session server, split out of #1519 for reviewability:
SessionServerSupervisorspawns ("spawn"context) N session workers + 1 thin router over socketpairs, gates readiness on the router's/healthreporting every worker healthy, fail-fasts on any child death, and tears the whole group down with no orphans. Nothing calls the supervisor yet — launch wiring and the removal of the single-process chassis land in the follow-up PRs.What's here
supervisor.py— spawn-context children (a forked child would inherit this Ray actor's threads/finalizers, e.g. wandb's service thread, and deadlock); the parent closes every socket end it holds so a child death is observable as EOF; readiness raises early if a child dies while loading its tokenizer; a monitor thread records the first death and tears the group down (SIGTERM → grace → SIGKILL,atexit-registered);check()re-raises the recorded failure for the rollout path.router.run_router— the router process entry point; imports nothing fromcore/worker, so the router process never loads the tokenizer/transformers stack.ipc.set_pdeathsig— the PR_SET_PDEATHSIG helper moves here (stdlib-only) so the router can use it too; the worker's private copy is replaced by the shared helper.tests/fast/router/test_session_supervisor.py— spawns real worker+router processes, drives create/chat/get end-to-end over IPC, then kills a worker and assertscheck()raises and no child is orphaned.Testing
pytest tests/fast/router— 96 passed at this commit (the in-process chassis is untouched here; the suite is green on this PR standing alone).🤖 Generated with Claude Code