Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the SGLang engine to treat both omitted tags and empty lists as targeting all components during sleep and wakeup operations. It also enhances the release tests by configuring a memory saver subprocess, parametrizing the sleep/wakeup tests to verify memory usage, and adding unit tests for state contracts using mocked components. The reviewer suggested safely accessing the LD_PRELOAD environment variable using os.environ.get to avoid potential KeyError exceptions.
|
|
||
| # Ray launches scheduler actors outside the engine's subprocess context. | ||
| with configure_subprocess(): | ||
| memory_saver_env = {"LD_PRELOAD": os.environ["LD_PRELOAD"]} |
There was a problem hiding this comment.
Accessing os.environ["LD_PRELOAD"] directly can raise a KeyError if the environment variable is not set (for example, on non-Linux platforms or if configure_subprocess() does not set it). It is safer to use os.environ.get("LD_PRELOAD") and only populate memory_saver_env if it is present.
| memory_saver_env = {"LD_PRELOAD": os.environ["LD_PRELOAD"]} | |
| ld_preload = os.environ.get("LD_PRELOAD") | |
| memory_saver_env = {"LD_PRELOAD": ld_preload} if ld_preload else {} |
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
1c5ea3e to
bb597ff
Compare
Keep Ray state consistent with acknowledged SGLang all-region wakeups. Cover CPU acknowledgement/failure contracts and real single-GPU sleep cycles with memory release, restoration, and deterministic output checks. The fixture configures TorchMemorySaver for Ray actors. AI-assisted implementation; human review and human-run tests remain required before requesting review. Signed-off-by: 0z5a <0z5a@users.noreply.github.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
bb597ff to
7f68ce3
Compare
Incremental review: Files changed against
master.Description
Clear Ray's tracked sleep state after an acknowledged
wakeup(tags=[]).SGLang treats both
Noneand an empty list as all memory regions, but Raypreviously cleared
_sleeping_tagsonly forNone. This leftis_sleeping()true after a successful full wakeup.
The production change is the all-tags condition plus its config documentation.
The existing SGLang release suite gains acknowledgement/error/cancellation
contracts and nine omitted/None/empty sleep-wakeup cycles. The GPU fixture now
configures TorchMemorySaver through the public
configure_subprocess()helperand Ray actor
runtime_env, enabling actual release/restoration checks.Related issues
Related to #62794; follows the control-plane implementation merged in #63021.
This does not implement ObjectRef weight transfer, sessions, an RL driver, or
the separate P/D builder in #63741.
Non-duplication: reread #62794's comments and searched open PRs for
62794 in:bodyand
sglang wakeupimmediately before submission; no same-fix PR was found. The bounded scope and
validation were already described in
the issue discussion.
Additional information
Tests
CPU contract selection (real Ray class/SGLang request types; only backend
tokenizer-manager calls mocked):
fixed 14 passed, zero skips. Three original failures are empty-tag wakeups;
the fourth is the subsequent selective-tag test observing their stale state.
python -m pytest release/llm_tests/serve/test_llm_serve_sglang.py \ -k '(sleep or pause or reset or test_sglang_serve_e2e or streaming or tokenize or batched) and not TestSGLangSleepState and not multi and not pipeline' \ -q --timeout=600The GPU harness initializes Ray with one GPU and substitutes a verified offline
checkpoint path before invoking this selection. All nine lifecycle cases check
at least 256 MiB release/restoration and exact greedy-output equality. Recorded
same-GPU process totals were approximately 38,384 -> 1,732 -> 38,384 MiB; these
are not allocator-level measurements. No backend calls are mocked in this run.
All applicable hooks passed on the final files. Validation uses Ray's documented
Python-only development setup: core wheel
dede511b61fbf6383f5b03cdcc1fa2c0128efafawith the checkout's LLM Python source, not a full Ray C++ source build. GPU
environment: Torch 2.13.0+cu130, SGLang 0.5.19, Transformers 5.12.1; model revision
7ae557604adf67be50417f59c2c2f167def9a775. Driver/containerLD_PRELOADwas unset;the fixture configures the actor environment. Initial dependency/preload setup
failures and an aborted macOS import-I/O rerun are not counted as passing tests
or bug reproductions. Multi-GPU/RL/performance qualification is outside scope.
AI assistance and human accountability
OpenAI Codex assisted with the audit, implementation, tests, and submission.
The human submitter confirmed reviewing every changed line and personally
running the relevant tests for commit
1c5ea3ee38ed10ec4eb84becc81e9cbeb9293135before this PR was opened. The commit's earlier pending-review wording records
its preparation-time status; the human confirmation was provided afterward.