Repository navigation
fix(vllm): remove worker sockets left behind by a process-group SIGTERM - #543
shipiyouniao merged 2 commits into
Conversation
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
left a comment
There was a problem hiding this comment.
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.
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
left a comment
There was a problem hiding this comment.
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.
|
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 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. |
Summary
A process-group SIGTERM leaves
/tmp/kvcached-tp-<name>-<hash>/behind with the last serve'sw<rank>.sockentries, 16 directories for 32 serves in the two-GPU validation of #482, which shipiyouniao filed as #510. The owner that already removes the/dev/shmsegment 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 fromWorkerProc.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 thenkill_process_treewithout 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.pygainsWorkerSocketCleanup. It remembers the deployment's worker sockets by(st_dev, st_ino)at capture,w<rank>.sockfor its TP ranks in each PP stage directory.unlink()removes a remembered node only while the node still has that identity and aconnect()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 fromtp_ipc_util, so the API server does not import the compiled extension for them.In
kvcached/integration/vllm/patches.py,_capture_engine_segmentcaptures the sockets as well if the owner has its parallel config (MPClient.vllm_config, or thevllm_configargument ofCoreEngineProcManager), and_unlink_stopped_engine_segmentreleases 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_pathdelegates the naming rule toutils. Stop, drain, retry andkvctl deletekeep their behavior.Validation
tests/test_vllm_engine_core_shutdown.pyadds 9 tests (24 cases) over the client, legacy client and supervisor owners. The process-group SIGTERM shape, engines reaped with-9and 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.pyadds 9 tests (11 cases) forWorkerSocketCleanupand 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
kvcachedandtests. Classification check valid.CPU only. For the GPU check, use the #482 pod protocol:
vllm servewith an explicitKVCACHED_IPC_NAMEat TP=2 and at PP=2, generate,kill -TERMthe 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