Skip to content

fix(vllm): remove worker sockets left behind by a process-group SIGTERM - #543

Merged
shipiyouniao merged 2 commits into
ovg-project:mainfrom
rishabhsinha17:fix/tp-socket-dirs-group-sigterm
Oct 9, 2026
Merged

shipiyouniao merged 2 commits into
ovg-project:mainfrom
rishabhsinha17:fix/tp-socket-dirs-group-sigterm

Conversation

@rishabhsinha17

@rishabhsinha17 rishabhsinha17 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A process-group SIGTERM leaves /tmp/kvcached-tp-<name>-<hash>/ behind with the last serve's w<rank>.sock entries, 16 directories for 32 serves in the two-GPU validation of #482, which shipiyouniao filed as #510. The owner that already removes the /dev/shm segment the killed engines leave (#482) now removes the worker sockets as well. It acts after the same confirmed engine exit, on the nodes it remembered at READY, and skips any node a worker still serves.

Root cause

The worker-side cleanup from #480 runs in Worker.shutdown(), which vLLM reaches from WorkerProc.shutdown() once the worker has closed its queues. On a process-group SIGTERM the API server stops its engine client at the same moment. With the 0.28 default shutdown timeout of 0 seconds its process manager sends the engine SIGTERM and then kill_process_tree without waiting, which SIGKILLs the workers first. Restart logs from the #482 run show the whole sequence inside one second, EngineCore still logging "waiting for worker exit count=2" right before the force kill, and no worker ever printed "IPC listener stopped". #519 handled the replacement-socket half of #510 and left this gap open on purpose, since the kill ends the worker before any hook in it runs.

Changes

kvcached/utils.py gains WorkerSocketCleanup. It remembers the deployment's worker sockets by (st_dev, st_ino) at capture, w<rank>.sock for its TP ranks in each PP stage directory. unlink() removes a remembered node only while the node still has that identity and a connect() to it fails with ECONNREFUSED. Unix socket nodes accept connections only while the process that bound them is alive, so a stopped or delayed worker keeps its socket, and a same-name replacement has a different inode and stays. Empty stage and root directories go afterwards, as in the worker-side stop, and only in the call that removed one of the deployment's nodes: vLLM calls the owner's shutdown more than once per teardown, and a completed cleanup is a no-op, so a same-name replacement that created the directory again between its makedirs() and bind() keeps it. Both the listener's identity helper and the empty-dir helper move here from tp_ipc_util, so the API server does not import the compiled extension for them.

In kvcached/integration/vllm/patches.py, _capture_engine_segment captures the sockets as well if the owner has its parallel config (MPClient.vllm_config, or the vllm_config argument of CoreEngineProcManager), and _unlink_stopped_engine_segment releases them after the segment, behind the existing engine-exit gate. Non-owning frontends and owners without a parallel config do not touch the sockets.

In kvcached/tp_ipc_util.py, get_worker_socket_path delegates the naming rule to utils. Stop, drain, retry and kvctl delete keep their behavior.

Validation

tests/test_vllm_engine_core_shutdown.py adds 9 tests (24 cases) over the client, legacy client and supervisor owners. The process-group SIGTERM shape, engines reaped with -9 and nodes left unserved, ends with no directory. Delayed workers keep their sockets until they exit, then the retry removes them. Same-name replacement nodes stay, live or dead. Unconfirmed engine exit keeps everything. Nodes absent at READY are never claimed. Non-owning frontends and owners without a parallel config leave the sockets as they were. Clients reuse the supervisor's capture. Repeated owner shutdowns leave a directory a replacement created after the first successful cleanup. Main fails all of these except the four preserve-only cases.

tests/test_tp_socket_cleanup.py adds 9 tests (11 cases) for WorkerSocketCleanup and the path rule, including a foreign rank's node, a probe that fails with a permission error, a replacement that creates its directory after a completed cleanup, and a retry that runs after the worker's own stop.

Full CPU manifest on macOS, Python 3.11: 1174 passed, failure set byte-identical to main debcb09 (the known shm, listener-device and pip environment cases), and the branch merges into that main without conflicts. ruff 0.11.7, isort 6.0.1, codespell 2.4.1 clean. CI-exact mypy 1.11.1 without torch, 3.10 to 3.13, clean on kvcached and tests. Classification check valid.

CPU only. For the GPU check, use the #482 pod protocol: vllm serve with an explicit KVCACHED_IPC_NAME at TP=2 and at PP=2, generate, kill -TERM the process group, wait for every process to exit, inspect /tmp/kvcached-tp-<name>-*. With the default shutdown timeout, main keeps the directory. I can rerun the 09-24 two-L40S matrix on request, and a T4 run of the same steps would settle it as well.

Fixes #510

A process-group SIGTERM leaves /tmp/kvcached-tp-<name>-<hash>/ with the
workers' sockets in it. The API server stops its engine client with the
0.28 default shutdown timeout of 0 seconds, and its process manager
SIGKILLs the engine tree before Worker.shutdown() runs in the workers.
The owner that removes the segment the killed engines leave now also
removes the worker sockets it remembered at READY, once it has confirmed
the engine exit and only for nodes that still have the remembered
identity and refuse a connect(). A stopped or delayed worker keeps its
socket, a same-name replacement has a different inode and stays, and
the owner removes empty stage and root directories afterwards.

Fixes ovg-project#510

Signed-off-by: Rishabh Sinha <rsinha17@terpmail.umd.edu>

@shipiyouniao shipiyouniao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent fallback fits #510. Independent Linux validation at c7232c7: 184 tests passed across both affected files. Process/socket probes confirmed live and stopped listener preservation, working protocol after connect-and-close, and cleanup after SIGTERM across TP/PP path layouts. GPU operations were stubbed; full vLLM serving and multi-GPU validation were not run. All nine exact-head CI checks are green. One repeated-cleanup issue below needs fixing.

Comment thread kvcached/utils.py Resolved
vLLM calls the owner's shutdown more than once per teardown, and every
call ran rmdir over the remembered directories even after the first one
had removed everything. A same-name replacement paused between its
makedirs() and bind() lost the directory it had just created and failed
to bind with ENOENT. A completed cleanup is now a no-op, a directory is
removed only by the call that removed one of the deployment's nodes from
it, and the deployment root is removed once empty like the worker's own
stop removes it. Retries for a socket a live worker still serves are
unchanged.

Signed-off-by: Rishabh Sinha <rsinha17@terpmail.umd.edu>

@shipiyouniao shipiyouniao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, the repeated-cleanup issue is fixed.

I tested 2edda2b with real Qwen2.5-0.5B weights and vLLM 0.28 on a T4 and two A800s. On the A800s, normal process-group SIGTERM and same-name restarts passed with TP=2 and PP=2 tested separately. The pre-PR control left dead sockets behind; this version removed them, including the PP stage directory. All nine CI checks are green.

Pausing one worker before SIGTERM also exposed an incomplete worker-exit case: two of three TP runs left live workers, whose sockets were correctly kept. The pre-PR control showed a similar case with PP=2. I'll follow up on the worker termination issue separately. Keeping a live worker's socket is the right behavior, so I don't think that needs to hold up this cleanup fix.

@shipiyouniao
shipiyouniao merged commit 043084a into ovg-project:main Oct 9, 2026
9 checks passed
@shipiyouniao

shipiyouniao commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

I tracked down the remaining live-worker case I said I'd investigate. It also reproduces without KVCached, on a single T4 with vLLM 0.28.0 and real Qwen2.5-0.5B-Instruct weights.

This was a shutdown fault-injection test: after successful requests, pause the worker, signal EngineCore, then send process-group SIGTERM once executor cleanup starts, with --shutdown-timeout 1. EngineCore resets SIGTERM to SIG_DFL before cleaning up its workers, so another SIGTERM can terminate it midway. The parent then sees EngineCore has exited and misses the orphaned worker. In the failing run, the worker remained alive and the API process did not exit within 45 seconds.

vllm-project/vllm#50529 already addresses this exact signal-handling window. I backported only that PR's signal-handling hunk onto an isolated 0.28.0 copy, leaving the process-manager code unchanged. The same test then passed twice without KVCached and once with KVCached: no live workers remained, and the KVCached socket directory was removed. None of those patched runs needed the test harness to kill leftover processes.

So this remaining case comes from vLLM's shutdown race, rather than an additional socket-cleanup defect here. I don't think we need another KVCached workaround for it; we can follow that upstream PR and wait for the fix. This is a targeted fault-injection result, not a claim that every shutdown failure is covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Worker socket directories remain after process-group SIGTERM

2 participants