config: the DP/EP topology reads come from the parallel bag - #35025
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f90fdcd4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| process owns the socket (polls zmq -> SHM) and every worker reads SHM. | ||
| """ | ||
| if server_args.tokenizer_worker_num > 1: | ||
| if get_serving().tokenizer_worker_num > 1: |
There was a problem hiding this comment.
Avoid serving-bag reads in the DP controller
When SGLANG_ROLE_NAMESPACES=enforce and ZMQ snapshots are active (multi-node DP attention or SGLANG_LOAD_SNAPSHOT_USE_ZMQ=1), DataParallelController calls this helper after publishing the dp_controller role. get_serving() then raises because runtime_context.py declares that role's allowed namespaces as only {"exec"}, so the controller fails during startup before creating its snapshot reader. Either avoid consulting the tokenizer owner for the DataParallelController caller or explicitly make the serving namespace part of that role's supported contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in cef7951: zmq_reader_owner now answers the DataParallelController caller from parallel-namespace leaves alone, so the controller path no longer reads the serving bag and its role set stays {"exec"}. An enforce-mode regression test in test/registered/unit/managers/test_load_snapshot_backends.py runs the owner check under SGLANG_ROLE_NAMESPACES=enforce for the controller caller.
e379947 to
b541fda
Compare
cef7951 to
c0c33cb
Compare
68a1e42 to
d01b96c
Compare
c0c33cb to
3b348b0
Compare
d01b96c to
a32135d
Compare
3b348b0 to
da39733
Compare
a32135d to
d9dd66f
Compare
da39733 to
9b2b9c2
Compare
d9dd66f to
7c68e7d
Compare
9b2b9c2 to
bb69559
Compare
7c68e7d to
518d41c
Compare
bb69559 to
7e9ba02
Compare
518d41c to
587d82d
Compare
5ba3b9b to
a6dc02f
Compare
587d82d to
afa022d
Compare
a6dc02f to
ddae0f2
Compare
afa022d to
ae3a7b2
Compare
ddae0f2 to
e15a852
Compare
ae3a7b2 to
da7c428
Compare
e15a852 to
ff264bc
Compare
da7c428 to
915adcc
Compare
ff264bc to
c4f7689
Compare
915adcc to
abe26a6
Compare
c4f7689 to
bbefd69
Compare
dp_size, enable_dp_attention and ep_size are resolution outputs: --dwdp-size derives all three, so a raw record carries the pre-derivation values. Every reader of the five topology fields now reads the parallel config leaves. configure_scheduler_process now reads ep_size from the parallel bag, so the scheduler process's publish must already have run when it is called. The load-snapshot owner decision and the scheduler recv skipper take no config argument any more -- every caller was handing over the process config. Their tests publish a config instead of injecting a stand-in object. The exposure pin drops 39 pairs on axis 1 and 19 on axis 2.
bbefd69 to
e12f01b
Compare
dp_size, enable_dp_attention and ep_size are resolution outputs: --dwdp-size
derives all three, so a raw record carries the pre-derivation values. Every
reader of the five topology fields now reads the parallel config leaves.
Two processes were reading configuration before they published: the scheduler
ran configure_scheduler_process ahead of its publish, and the launch path
published only once the tokenizer manager was built. Both publish at process
entry now.
The load-snapshot owner decision and the scheduler recv skipper take no config
argument any more -- every caller was handing over the process config. Their
tests publish a config instead of injecting a stand-in object.
The exposure pin drops 39 pairs on axis 1 and 19 on axis 2.
CI States
Latest PR Test (Base): 🚫 Run #32079795399
Latest PR Test (Extra): 🚫 Run #32079795182