Skip to content

[PD] Gate disagg event loops on _engine_paused; reject pause(retract) in PD - #23672

Merged
ByronHsu merged 1 commit into
sgl-project:sglang-milesfrom
ByronHsu:byron/pd-pause-fix
Apr 27, 2026
Merged

[PD] Gate disagg event loops on _engine_paused; reject pause(retract) in PD#23672
ByronHsu merged 1 commit into
sgl-project:sglang-milesfrom
ByronHsu:byron/pd-pause-fix

Conversation

@ByronHsu

@ByronHsu ByronHsu commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Why?

In PD-disaggregated mode, pause_generation does not actually quiesce the engine and can leave the scheduler in an inconsistent state:

  1. The disagg event loops (event_loop_normal_disagg_prefill, event_loop_overlap_disagg_prefill, event_loop_normal_disagg_decode, event_loop_overlap_disagg_decode) never check self._engine_paused. They keep popping bootstrap queues, draining KV transfer queues, building batches, and running forwards while the engine is supposedly paused. As a result, mode="in_place" is effectively a no-op in PD and cannot serve as a weight-sync barrier.

  2. mode="retract" partially retracts decode-side requests into disagg_decode_prealloc_queue, but there is no mechanism to send them back to the prefill node for re-prefill. A subsequent flush_cache then crashes inside RadixCache.dec_lock_ref because requests still in disagg_prefill_inflight_queue hold tree locks across the radix-tree reset:

File "sglang/srt/mem_cache/common.py", line 479, in release_kv_cache
    tree_cache.cache_finished_req(req, is_insert=is_insert)
File "sglang/srt/mem_cache/radix_cache.py", line 508, in cache_finished_req
    self.dec_lock_ref(req.last_node)
File "sglang/srt/mem_cache/radix_cache.py", line 642, in dec_lock_ref
    assert node is self.root_node, \
        "This request holds the node from another tree"

How?

  • Add if self._engine_paused: continue immediately after process_input_requests in both PD prefill and PD decode event loops, mirroring the unified-mode pattern. Control RPCs continue to be processed while paused, but no scheduler-visible state transitions occur until continue_generation.
  • Assert early in Scheduler.pause_generation that mode="retract" is unsupported when disaggregation_mode != NULL, before mutating any scheduler state. A proper fix requires a decode-to-prefill rebootstrap protocol that does not exist yet; until then, failing fast is preferable to partial state changes followed by a delayed crash inside radix-cache cleanup.
  • Update the pause_generation unit-test fixture to set disaggregation_mode = NULL so the existing retract-mode tests keep exercising the unified-mode path.

Made with Cursor

… in PD

In PD-disaggregated mode, pause_generation does not actually quiesce the
engine and can leave the scheduler in an inconsistent state:

1. event_loop_normal_disagg_prefill, event_loop_overlap_disagg_prefill,
   event_loop_normal_disagg_decode, and event_loop_overlap_disagg_decode
   never check self._engine_paused. They keep popping bootstrap queues,
   draining KV transfer queues, building batches, and running forwards
   while the engine is supposedly paused. As a result, mode="in_place"
   is effectively a no-op in PD and cannot serve as a weight-sync
   barrier.

2. mode="retract" partially retracts decode-side requests into
   disagg_decode_prealloc_queue, but there is no mechanism to send them
   back to the prefill node for re-prefill. A subsequent flush_cache
   then crashes inside RadixCache.dec_lock_ref because requests in
   disagg_prefill_inflight_queue still hold tree locks across the
   radix-tree reset.

This patch:

- Adds `if self._engine_paused: continue` immediately after
  process_input_requests in both PD prefill and PD decode event loops,
  mirroring the unified-mode pattern. Control RPCs continue to be
  processed while paused, but no scheduler-visible state transitions
  occur until continue_generation.

- Asserts early in Scheduler.pause_generation that mode="retract" is
  unsupported when disaggregation_mode != NULL, before mutating any
  scheduler state. A proper fix requires a decode-to-prefill
  rebootstrap protocol; until then, failing fast is preferable to
  partial state changes followed by a delayed crash inside radix-cache
  cleanup.

- Updates the pause_generation unit test fixture to set
  disaggregation_mode = NULL so existing retract-mode tests keep
  exercising the unified-mode path.

Made-with: Cursor
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Comment on lines +434 to 438
if self._engine_paused:
continue
self.waiting_queue.extend(
self.disagg_prefill_bootstrap_queue.pop_bootstrapped()
)

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.

Should we bypass pop_bootstrapped on the prefill side? Will it cause the bootstrapping timeout for some requests when pausing generation?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think this is unlikely to happen for now because timeout is 300s but weight sync usually under 30s. I will follow up if i meet similar issue in real run.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

#23886 addressed here and fixed the conflicts

@ShangmingCai ShangmingCai 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.

Others look good.

@ByronHsu
ByronHsu merged commit 1accc43 into sgl-project:sglang-miles Apr 27, 2026
6 checks passed
XinyuJiangCMU pushed a commit to XinyuJiangCMU/sglang that referenced this pull request Jul 17, 2026
…gl-project#23887)

Kept only the net-new delta on v0.5.15: fail-fast assertion in
pause_generation + unit-test disaggregation_mode setup. The decode.py
whitespace and inline test_pause_resume_in_place are dropped (v0.5.15
covers them via PauseResumeInPlaceMixin).
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.

2 participants