Repository navigation
[Frontend][RL] Align sleep-mode API responses and operation metrics - #52864
Conversation
|
@aoshen02 could you please review this pr,thanks! |
|
This pull request has merge conflicts that must be resolved before it can be |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 6fbb00b and 8ebc3072e3b4f3b0f1795f1c8e7d1fe4d14ebffd. 📒 Files selected for processing (13)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. WalkthroughThe changes add structured sleep, wake, and weight-version responses with RLHF lifecycle tests. They also improve RLHF server helpers and fix independent Mooncake scheduling, MXFP4 backend selection, and ROCm initialization test coverage. ChangesRLHF HTTP lifecycle contracts
Mooncake load scheduling
MXFP4 backend selection
ROCm initialization coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RLHFTestClient
participant SleepAPI
participant EngineClient
RLHFTestClient->>SleepAPI: POST sleep or wake request
SleepAPI->>EngineClient: Apply requested state transition
EngineClient-->>SleepAPI: Return engine state
SleepAPI-->>RLHFTestClient: Return status, tags, and elapsed time
Merge Risk: ⚪ Minimal · up to The updated lifecycle responses, validation, scheduler handling, backend fallback, and test coverage do not leave an identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation Several changes do not relate to the stated sleep API, metrics, or RLHF information-retrieval objectives. These include Mooncake scheduler changes, DeepSeek MoE backend changes, and the ROCm initialization skip. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tics Signed-off-by: Ronald1995 <ronaldautomobile@163.com>
H20 verification — revision 2 (
|
- Drop the operations counter: http_requests_total already counts each weight route by status, matching the sleep-mode metrics in vllm-project#52864. - Without the counter, the init_info/update_info type checks, the separate set_version operation, and the success/error status have no purpose; restore the original finish_weight_update call. - Replace four Python test files with one covering the recorder, every route, rejected input, and lazy registration on the default registry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
The -> bool annotation already states the return contract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
- Drop the runtime bool checks on wake_up results; the -> bool annotation states the contract. The DP client now just returns all() of every engine's result. - Rust: go back to Query<SleepParams> plus a level <= 2 check instead of a hand-written query parser, and let the metric guards drop at scope end. - Remove tests for the dropped checks and for pure return-value pass-through; shorten the new docstrings to one line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
Add AsyncMPClient.call_utility_all_async, the counterpart of the Rust client's call_utility: one result per managed engine. wake_up_async reduces it with all() once in the base client instead of a DPLB-only override, and DPLB's call_utility_async reuses it to keep engine 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
Add call_utility_consensus_async next to call_utility_all_async and use them wherever the result depends on every DP engine, matching the Rust client: reset_prefix_cache, add_lora, remove_lora and pin_lora succeed only if every engine does; is_sleeping, is_scheduler_paused and get_weight_version require all engines to agree. They previously reported engine 0 only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
This reverts commit 711c9e6. Keep this PR to wake_up; aligning the other utilities with the Rust client moves to a separate PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
This reverts commit c8dfcaf. wake_up stays in vllm-project#52864. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
|
This pull request has merge conflicts that must be resolved before it can be |
Drop the call_utility_all_async copy now that vllm-project#59240 landed it; this PR only switches wake_up_async to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com>
|
/ci run |
|
✅ @Ronald1995, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #91995 for commit |
|
/ci run |
|
✅ CI is already running for this commit: https://buildkite.com/vllm/ci/builds/91995 |
|
/ci retry |
|
✅ Queued 1 failed job(s) for retry in Buildkite CI #91995. |
CI selector (shadow): 178 test steps (253 jobs) instead of 76 (92 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Selector would run (178)
Would skip (today's rules run them) (2)
Would add (today's rules do not run them) (104)
AMD mirrors: would skip (1)
AMD mirrors: would add (86)
CI results (2026-09-30 14:41 UTC)165 passed, 0 failed, 1 pending. No failures to judge. 14 changed files · base |
…llm-project#52864) Signed-off-by: Ronald1995 <ronaldautomobile@163.com> Signed-off-by: aoshen02 <aoshen02@users.noreply.github.com> Co-authored-by: aoshen02 <aoshen02@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Scope
This PR aligns the Python and Rust sleep API contract and adds the operation observability needed for long sleep/wake operations.
POST /sleep:{"status":"sleeping","level":0|1|2}.POST /release_kv_cache_memory:{"status":"kv_cache_released"}.POST /wake_up:{"status":"awake"|"sleeping","tags":null|[...]}.tagsechoes the requested tags; null means wake all. Status uses the engine'sfully_awakeresult, without a second state query.EngineClient.wake_up()now returns bool (fully awake);LLM.wake_up()propagates it. Interface change: out-of-treeEngineClientimplementations must return bool, since a returned None would read as "sleeping".wake_upnow reports fully awake only if every engine is. It reusescall_utility_all_asyncfrom [Core] Combine per-engine utility results like the Rust client #59240.Metrics and review response
Request counts and HTTP status use the existing
http_requests_total{handler,method,status}. The redundant sleep operation counter is removed. Rust generic HTTP middleware now includes/sleep,/release_kv_cache_memory,/wake_up, and/is_sleeping, matching Python.Two sleep-specific metrics remain, with no
rl_prefix:vllm:sleep_mode_operation_duration_seconds{operation}: buckets suitable for long operations; operations are sleep, release_kv_cache_memory, and wake.vllm:sleep_mode_operations_in_flight{operation}: detects outstanding sleep/wake work; cleaned up on success, error, and cancellation.No global HTTP in-progress middleware redesign or released/restored memory byte metrics are included. The existing engine sleep-state model is outside this PR; resource dimensions for level-0, partial wake, and KV release are implemented separately in draft follow-up #59130.
Testing
Current revision (
47e7f39408):git diff --check./metricsscraping.Previously verified Linux/H20 results at
403f4b348f, before these review changes (test report):Rebased onto main
aedaba8664. All commits include Signed-off-by. AI assistance was used; review and Linux CI of the current revision are needed before merge.