[Bugfix] Fix example disagg_example_p2p_nccl_xpyd.sh zombie process #21437
Conversation
Signed-off-by: David Chen <[email protected]>
![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 correctly identifies and fixes an issue with a zombie process by explicitly terminating it during cleanup. My feedback suggests a minor but important improvement: using a more graceful termination signal (SIGTERM instead of SIGKILL) to ensure the proxy server can clean up its resources properly, which enhances the overall robustness of the example script.
| @@ -93,6 +93,7 @@ ensure_python_library_installed() { | |||
| cleanup() { | |||
| echo "Stopping everything…" | |||
| trap - INT TERM # prevent re-entrancy | |||
| pkill -9 -f "disagg_proxy_p2p_nccl_xpyd.py" | |||
There was a problem hiding this comment.
Using SIGKILL (-9) should be a last resort, as it terminates the process immediately without allowing it to perform any cleanup operations. The disagg_proxy_p2p_nccl_xpyd.py script runs a Quart server, which can handle a graceful shutdown on SIGTERM (the default signal for pkill) to properly close network sockets and other resources.
Forcibly killing the server with SIGKILL might leave resources in an inconsistent state, potentially causing issues like "Address already in use" errors if you run the example again quickly.
I recommend removing the -9 flag to allow the process to shut down gracefully.
| pkill -9 -f "disagg_proxy_p2p_nccl_xpyd.py" | |
| pkill -f "disagg_proxy_p2p_nccl_xpyd.py" |
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
|
@DarkLight1337 @KuntaiDu please review,thanks |
| @@ -93,6 +93,7 @@ ensure_python_library_installed() { | |||
| cleanup() { | |||
| echo "Stopping everything…" | |||
| trap - INT TERM # prevent re-entrancy | |||
| pkill -9 -f "disagg_proxy_p2p_nccl_xpyd.py" | |||
github-actions
bot
added
the
ready
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: 董巍 <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: 董巍 <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: avigny <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: shuw <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: x22x22 <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: Jinzhen Lin <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: Paul Pak <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: Boyuan Feng <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]> Signed-off-by: Diego-Castan <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]>
…llm-project#21437) Signed-off-by: David Chen <[email protected]>
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
fix #21432 ，After disagg_example_p2p_nccl_xpyd.sh cleanup, there is a zombie process disagg_proxy_p2p_nccl_xpyd.py.
In order to become an independent daemon process, the disagg_proxy_p2p_nccl_xpyd.py script performs an operation similar to os.setsid() in the code, which creates a new process group. As a result, it no longer belongs to the process group of the original script, so the kill -- -$ command cannot affect it, causing it to remain after the script ends.
Test Plan
bash vllm/examples/online_serving/disaggregated_serving_p2p_nccl_xpyd/disagg_example_p2p_nccl_xpyd.sh
Test Result
running:
result:
after cleanup:
(Optional) Documentation Update