Graceful shutdown with SIGTERM for child processes - #16484
chenkaiyue wants to merge 12 commits into
Conversation
Summary of ChangesHello @chenkaiyue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the graceful shutdown mechanism within SGLang, particularly for child processes like hicache, which require time for proper resource cleanup. By introducing a phased termination approach that prioritizes SIGTERM before resorting to SIGKILL, and by refining how exit signals are handled in key managers, the system can now perform necessary cleanup operations, such as executing Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a crucial feature for graceful shutdown of child processes, which significantly improves server stability and resource management in containerized environments. The approach of sending SIGTERM before SIGKILL is well-implemented in the new graceful_kill_process_tree function. The changes in tokenizer_manager to use this new function and os._exit(0) are correct and well-reasoned. Similarly, adding a SIGTERM handler in the scheduler process is a good move for ensuring proper cleanup. My review includes a few suggestions to enhance code quality by removing redundant code and simplifying some logic.
f075452 to
1ba9484
Compare
780e3c3 to
98b321e
Compare
… propagation) When Kubernetes sends SIGTERM to SGLang pods running PD disaggregation, the sigterm_watchdog previously sent SIGKILL to scheduler child processes, bypassing all Python cleanup (atexit, __del__, C++ destructors). This left Mooncake/NIXL RDMA memory regions registered, causing pod sandbox teardown to hang with FailedKillPod errors. Parent side (tokenizer_manager): - Add graceful_kill_process_tree() that sends SIGTERM first, waits up to a configurable timeout (SGLANG_CHILD_PROCESS_SHUTDOWN_TIMEOUT, default 10s), then SIGKILL for stragglers - Use os._exit(0) instead of sys.exit(0) to avoid SystemExit being caught by the asyncio event loop Child side (scheduler + KV managers): - Register SIGTERM handler in scheduler that calls sys.exit(143) so atexit handlers run instead of the default immediate termination - Register atexit handler that calls kv_manager.shutdown() for RDMA cleanup - Add CommonKVManager.shutdown() that closes ZMQ socket (linger=0) and terminates context, unblocking threads stuck on recv_multipart() - Add MooncakeKVManager.shutdown() that shuts down thread pool executors then deregisters all RDMA memory (kv, aux, state buffers) - Add NixlKVManager.shutdown() that deregisters RDMA memory via NIXL agent - Mark bootstrap_thread, decode_thread, heartbeat_checker as daemon=True Combines sgl-project#16484 and sgl-project#19810. Co-Authored-By: chenkaiyue <chenkaiyue2008@163.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
951c765 to
3744b9a
Compare
Validation report — graceful shutdown verified end-to-end (with raw logs)Supplementing the description with the actual evidence. Validated on a production TKE deployment: Scenario A — graceful SIGTERM (TP8 + Mooncake L3,
|
| propagation log | scheduler handlers ran | exit | |
|---|---|---|---|
| Patched | present | 8 / 8 | 0 |
Baseline (only data_parallel_controller.py reverted) |
absent | 0 / 8 | schedulers SIGKILLed via PR_SET_PDEATHSIG |
# PATCHED
SIGTERM received in data_parallel_controller; propagating graceful shutdown to scheduler children...
scheduler SIGTERM handler count = 8
RUN_EXIT label=Cpatched code=0
# BASELINE (un-patched data_parallel_controller.py): schedulers die in ~3s, before any handler runs
t=1s main_alive=yes sched_alive=8
t=2s main_alive=yes sched_alive=7
t=3s main_alive=yes sched_alive=2
t=4s main_alive=yes sched_alive=0
propagating graceful shutdown : 0 matches
SIGTERM received in scheduler process : 0 matches
Without the fix, none of the 8 schedulers run any cleanup — they are SIGKILLed before processing SIGTERM.
Scenario D — real Kubernetes-recorded container exit code (137 → 0)
A patched image (FROM sglang:v0.5.12-cu129 + the 4 files) was run as the Deployment with the sglang
server as the container PID-1 process (exec). SIGTERM was delivered as kubelet does on pod termination
(terminationGracePeriodSeconds=180), and the exit code was read from the pod containerStatuses:
| image | K8s-recorded container exit |
|---|---|
| patched (this PR) | exitCode=0, reason=Completed |
| baseline (stock) | exitCode=137, reason=Error |
On a real scale-to-0 pod delete the patched pod also terminated gracefully over ~54s (within the grace
period) instead of being force-killed.
Summary
| scenario | result |
|---|---|
| A — graceful (TP8 + Mooncake L3) | exit 0; 8/8 handlers; 8/8 removeSegmentDesc; segments 8→0; no watchdog false-crash |
| A (timeout=10) — SIGKILL fallback | timeout→SIGKILL branch fires; still exits 0 |
| A2 — force path | immediate kill; exit 137 (intentional) |
| C — DP controller propagation | patched 8/8 handlers + exit 0 vs baseline 0/8 (pdeathsig SIGKILL) |
| D — real K8s exit code | patched 0/Completed vs baseline 137/Error |
Takeaway for operators: under RDMA, full freeEngine teardown can exceed 10s, so set
SGLANG_CHILD_PROCESS_SHUTDOWN_TIMEOUT generously (e.g. 60) and below terminationGracePeriodSeconds.
This change is complementary to kvcache-ai/Mooncake#1363 (master-side cleanup of HTTP metadata on client
heartbeat timeout, the crash path); together no stale Mooncake metadata is left in any shutdown scenario.
e0e0fa5 to
ae28d4a
Compare
How this differs from #28779, and why it's still needed#28779 (merged) and this PR are complementary layers; this PR is rebased on top of it. #28779 — in-band, scheduler-only. A This PR — the outer layer. After sending
Why DP-attention still needs the signal handlers (the subtle part). ValidationProd TKE,
Takeaways: the parent exits |
Follow-up: deterministic evidence that the DP controller handler is load-bearingThe validation table above uses the real hicache config, where the Same prod box (
Mechanism: in both runs all 8 ranks receive The parent process still exits |
d8cd152 to
f945405
Compare
|
/tag-and-rerun-ci |
|
/rerun-failed-ci |
CI status: only
|
| when | failing test / step | root cause | origin |
|---|---|---|---|
| earlier | test_norm.py |
sgl_kernel missing fused_qk_gemma_rmsnorm_cpu |
main packaging (log) |
| earlier | test_spec_eagle_topk_cpu.py |
HF 401 on gated meta-llama/Llama-2-7b-chat-hf |
infra/auth, cleared on rerun (log) |
| earlier | run_suite.py |
HWBackend has no attribute 'MLX' |
version skew from #30121, fixed after I re-merged main (log) |
| now | test_intel_amx_attention_backend_b.py |
TypeError: grouped_topk_cpu() got an unexpected keyword argument 'scoring_func' |
Python↔sgl_kernel signature skew (log) |
Current failure (TestIntelAMXAttnBackendQuant.test_latency_fp8_moe_model):
File ".../sglang/srt/layers/moe/topk.py", line 2024, in select_experts
topk_weights, topk_ids = grouped_topk(...)
TypeError: grouped_topk_cpu() got an unexpected keyword argument 'scoring_func'
The Python MoE top-k path now passes scoring_func into the CPU kernel, but the grouped_topk_cpu() binding on the CPU lane does not accept it — a main-side Python/kernel mismatch that fails independently of this PR.
Whether build-test (xeon-gnr, base-b-test-cpu) is a required gate for this PR? If it's the known-broken main CPU lane, this PR is otherwise ready (GPU gates green, changes unrelated to the CPU path). Happy to re-merge main once the CPU lane is fixed upstream.
|
/rerun-failed-ci |
3 similar comments
|
/rerun-failed-ci |
|
/rerun-failed-ci |
|
/rerun-failed-ci |
… scheduler, DP controller) On SIGTERM, send SIGTERM to child processes first so they can run their cleanup (atexit / __del__ / C++ destructors, e.g. Mooncake/hicache RDMA teardown), then SIGKILL only stragglers, and let the scheduler / data_parallel_controller exit normally. Kubernetes then records exit 0 (Completed) instead of 137, and child resources are released instead of leaked. - utils/common.py: add graceful_kill_process_tree() (SIGTERM -> poll /proc -> SIGKILL stragglers; avoids psutil.wait_procs OSError(EINVAL) on grandchildren) and get_child_process_shutdown_timeout() / SGLANG_CHILD_PROCESS_SHUTDOWN_TIMEOUT. - managers/tokenizer_manager.py: stop SubprocessWatchdog before terminating children, then graceful_kill_process_tree() + os._exit(0); keep force paths (UnHealthy / SGL_FORCE_SHUTDOWN) as immediate kill_process_tree. - managers/scheduler.py: register SIGTERM handler that sys.exit(0) (exit 0 keeps SubprocessWatchdog from treating it as a crash), with SIG_IGN reentry guard. - managers/data_parallel_controller.py: register SIGTERM handler that propagates graceful shutdown to scheduler children (they would otherwise be SIGKILLed via PR_SET_PDEATHSIG before cleanup). Co-authored-by: Cursor <cursoragent@cursor.com>
…tree) Co-authored-by: Cursor <cursoragent@cursor.com>
- scheduler: set gracefully_exit in the SIGTERM handler so the finally block runs release_host_resources() (hisparse teardown), matching the ShutdownReq path. The SIGTERM safety-net path previously skipped it, leaking pinned host buffers / RDMA segments when a scheduler was SIGTERM'd (slow RDMA past the ShutdownReq wait, or DP propagation) instead of exiting via ShutdownReq. - tokenizer_manager: wrap the ShutdownReq wait + graceful_kill in soft_watchdog.disable() so a small soft_watchdog_timeout can't fire SIGQUIT and force-kill the tree mid-cleanup; make the ShutdownReq wait configurable via SGLANG_SCHEDULER_SHUTDOWN_TIMEOUT (was hardcoded 15s). - common: add get_scheduler_shutdown_wait_timeout() + SGLANG_SCHEDULER_SHUTDOWN_TIMEOUT (default 15s); note the terminationGracePeriodSeconds constraint.
Move SGLANG_CHILD_PROCESS_SHUTDOWN_TIMEOUT and SGLANG_SCHEDULER_SHUTDOWN_TIMEOUT into Envs as EnvFloat fields, accessed via envs.<NAME>.get(), instead of the raw os.environ.get wrappers in common.py. Matches sglang's env-var governance (environ.py) and the existing SGLANG_FORCE_SHUTDOWN / SGLANG_REQ_RUNNING_TIMEOUT pattern.
Extract the common graceful-shutdown skeleton (SIG_IGN re-entry guard, log, optional cleanup hook, sys.exit(0)) into install_graceful_sigterm_handler() in utils/common.py. The scheduler and data_parallel_controller now pass their process-specific action as a callback, keeping behavior identical.
9888a4d to
a74f73f
Compare
|
/rerun-failed-ci |
|
/rerun-failed-ci |
|
@hnyls2002 Could you please help review this PR? |
Motivation
When SGLang receives
SIGTERM(e.g. a Kubernetes pod deletion / rolling update), the currentTokenizerManager.sigterm_watchdog()callskill_process_tree(os.getpid(), include_parent=True), whichSIGKILLs every process including itself. Two problems:
exitCode 137, whichlooks like a crash/OOM in dashboards and restart policies.
hicache Mooncake storage backend (RDMA memory regions, segment descriptors, RPC metadata) — rely on
Python
atexithandlers,__del__, and C++ destructors (PyClient::~PyClient()→tearDownAll_internal(),~TransferEngine()→freeEngine()). SIGKILL bypasses all of them, leakingRDMA registrations and stale metadata, and can hang pod sandbox teardown.
This PR makes shutdown send SIGTERM first (so children run their cleanup), escalate to SIGKILL only for
stragglers, and lets the scheduler / data parallel controller exit normally so their cleanup runs.
Modifications
python/sglang/srt/utils/common.pygraceful_kill_process_tree(): SIGTERM all children → wait up to a timeout (polling/procviais_running()/status(), notpsutil.wait_procs, whosepidfd_openpath can raiseOSError(EINVAL)against recursively-collected grandchildren on some kernels) → SIGKILL stragglers.get_child_process_shutdown_timeout()/SGLANG_CHILD_PROCESS_SHUTDOWN_TIMEOUTenv (default 10s).python/sglang/srt/managers/tokenizer_manager.pysigterm_watchdog()stops theSubprocessWatchdogbefore terminating children — otherwise itobserves a child exit (e.g. the detokenizer dying on the default SIGTERM with exit code -15) and
misclassifies the graceful shutdown as a crash, firing
SIGQUIT→ SIGKILL of the whole tree.graceful_kill_process_tree(include_parent=False, timeout=...)andos._exit(0)(a plainsys.exitwould be swallowed by the asyncio event loop running the coroutine).server_status == UnHealthy,SGL_FORCE_SHUTDOWN) keep the old immediatekill_process_tree(include_parent=True)(no graceful wait).python/sglang/srt/managers/scheduler.pySIGTERMhandler inrun_scheduler_process()thatsys.exit(0)soatexit/__del__/C++ destructors run. Exit code 0 is intentional:
SubprocessWatchdogtreats a non-zero / by-signal childexit as a crash. First line re-arms
SIGTERMtoSIG_IGNso a re-propagated signal cannot re-enter thehandler and interrupt cleanup.
python/sglang/srt/managers/data_parallel_controller.pySIGTERMhandler that propagates graceful shutdown to its scheduler children viagraceful_kill_process_tree(), thensys.exit(0). In DP mode the schedulers are children of thecontroller, which sets
PR_SET_PDEATHSIG=SIGKILL; without this the controller would die immediately andthe kernel would SIGKILL the schedulers mid-cleanup.
Validation
Validated end-to-end on a production TKE deployment (
MiniMax-M2.7,sglang:v0.5.12-cu129, TP8/EP8,HCCPNV6 8×GPU + 8×
mlx5_bondRoCE), withmooncake_master(HTTP metadata server) running in-process andthe hicache Mooncake L3 backend enabled.
Graceful shutdown (SIGTERM, TP8 + Mooncake L3): process exit code 0; all 8 scheduler ranks logged
SIGTERM received in scheduler process (TPx PP0); exiting normally...; all 8 Mooncake segment descriptorsremoved by the transfer-engine destructor (
removeSegmentDesc ... finish) and master-mounted segmentsdropped 8→0 with immediate
UnmountSegment status=success; noSubprocessWatchdogfalse-crash.SIGKILL straggler fallback (
timeout=10):... did not terminate within 10.0s, sending SIGKILL: pids=[...]fires and the process still exits 0 (set the timeout generously under RDMA and belowterminationGracePeriodSeconds).Force path (
SGL_FORCE_SHUTDOWN=1):Force exiting.with zero graceful-kill logs and exit code 137 —the intentional immediate kill, a clean contrast to the graceful path's 0.
DP controller propagation (
--dp-size 8 --enable-dp-attention), A/B:data_parallel_controller.pyreverted)PR_SET_PDEATHSIGwithin ~3sReal Kubernetes-recorded container exit code (patched image vs stock image, server as container PID 1,
SIGTERM as kubelet delivers it,
terminationGracePeriodSeconds=180):exitCode=0, reason=CompletedexitCode=137, reason=ErrorThis is complementary to kvcache-ai/Mooncake#1363 (master-side cleanup of HTTP metadata on client
heartbeat timeout), which covers the crash path; together no stale Mooncake metadata is left in any
shutdown scenario.
See Validation report here: #16484 (comment)
CI States
Latest PR Test (Base): ❌ Run #31763700004
Latest PR Test (Extra): ❌ Run #31763699789