-
Notifications
You must be signed in to change notification settings - Fork 8.9k
[PD] Graceful shutdown for disaggregation with RDMA cleanup #22200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| # ============================================================================== | ||
| """A scheduler that manages a tensor parallel GPU worker.""" | ||
|
|
||
| import atexit | ||
| import faulthandler | ||
| import logging | ||
| import os | ||
|
|
@@ -3609,6 +3610,35 @@ def run_scheduler_process( | |
| # Send initialization info back to the parent process | ||
| pipe_writer.send(scheduler.get_init_info()) | ||
|
|
||
| # Register RDMA cleanup for disaggregation modes only. | ||
| # This ensures RDMA memory is deregistered and ZMQ sockets are closed | ||
| # when the scheduler process exits, preventing Kubernetes pod sandbox | ||
| # teardown hangs (FailedKillPod). | ||
| if scheduler.disaggregation_mode != DisaggregationMode.NULL: | ||
|
|
||
| def _shutdown_kv_manager(): | ||
| try: | ||
| kv_mgr = None | ||
| if hasattr(scheduler, "disagg_prefill_bootstrap_queue"): | ||
| kv_mgr = scheduler.disagg_prefill_bootstrap_queue.kv_manager | ||
| elif hasattr(scheduler, "disagg_decode_prealloc_queue"): | ||
| kv_mgr = scheduler.disagg_decode_prealloc_queue.kv_manager | ||
| if kv_mgr is not None and hasattr(kv_mgr, "shutdown"): | ||
| kv_mgr.shutdown() | ||
| except Exception as e: | ||
| logger.warning(f"Error during KV manager shutdown: {e}") | ||
|
|
||
| atexit.register(_shutdown_kv_manager) | ||
|
Comment on lines
+3619
to
+3631
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The _shutdown_kv_manager function is defined inside the loop and registered with atexit. If run_scheduler_process is called multiple times or if the scheduler is re-initialized, this could lead to multiple atexit registrations. It is better to register the cleanup handler once or ensure it is cleaned up properly. |
||
|
|
||
| # Register SIGTERM handler so atexit handlers run on graceful shutdown. | ||
| # Without this, SIGTERM uses the default handler (immediate exit) which | ||
| # skips atexit/destructors, leaving RDMA resources unreleased. | ||
| def _sigterm_handler(signum, frame): | ||
| logger.info("Scheduler received SIGTERM, exiting gracefully...") | ||
| sys.exit(143) # 128 + SIGTERM(15) | ||
|
|
||
| signal.signal(signal.SIGTERM, _sigterm_handler) | ||
|
|
||
| # Run the event loop (blocks until shutdown) | ||
| scheduler.run_event_loop() | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shutdown method should ideally be idempotent. If shutdown is called multiple times, the second call might raise an exception when attempting to close the socket or terminate the context again. Adding a check to ensure the socket and context are still valid or using a flag would make this more robust.