Repository navigation
fix(model-deployment): route External DP members and add walkthroughs - #752
Conversation
|
🔍 OpenCodeReview found 6 issue(s) in this PR.
📄
|
|
Addressed the one summary-routed documentation finding: the shared-memory paragraph now records measured mount capacity and links to the configuration guide for the default rule. There were no top-level reviews or inline threads to resolve. The walkthrough now uses the official GPUStack runner throughout. Its unchanged vLLM 0.29.0 / Ray 2.54.0 installation passed elastic width 2→4 with TP=2, native DP 2→4 and EP world 4→8. Ordinary/SSE requests succeeded before and after expansion; the master Pod and HTTP process were retained. The image contains only the CUDA 13 CuPy package. Captured CR projections match the official run. The obsolete field discussion has been removed. Verification: documentation lint, server dry-run for all YAML examples, current-generation observation checks, and code generation with a clean generated tree. The existing full race-enabled test run passed. This round uses a fixup commit; squash merge will absorb it. |
952d26b to
faf20fe
Compare
Signed-off-by: thxCode <thxcode0824@gmail.com>
Signed-off-by: thxCode <thxcode0824@gmail.com>
|
Reviewed all feedback buckets: 3 top-level reviews (marker-only bodies), 5 inline findings, and 2 issue comments. The current summary contains 2 additional findings; the earlier summary-only shared-memory wording finding was already addressed.
Primary engine evidence: CLI mode validation and LB normalization. The two documentation findings are addressed. The three disputed inline findings receive source-based replies and remain open for reviewer judgment. Full product tests, code generation, and the runtime tests passed. The final documentation pass checks the reorganized site and its exports. The branch's existing fixup commit is folded as part of final history cleanup. |
20d674b to
c0bca4f
Compare
Signed-off-by: thxCode <thxcode0824@gmail.com>
Signed-off-by: thxCode <thxcode0824@gmail.com>
|
This review emitted 7 findings: 3 inline, 3 summary-routed, and 1 skipped as an overlap with earlier feedback. All current inline and summary findings were read.
The compatibility decisions are scope choices, not claims that the legacy concerns are impossible. The skipped overlap was also read from the review log and matched to the earlier response. The GPU runtime record remains the tested operator binary |
c0bca4f to
a76328a
Compare
What type of PR is this?
/kind bug
/kind documentation
/area worker
What this PR does / why we need it:
A managed Router registered only member 0 of an External DP Server group, even though every rank served HTTP. Remove that leader restriction for qualified External Server endpoints while preserving endpoint eligibility. Internal and Prefill/Decode groups keep their leader restriction, including mixed-role deployments. Peer discovery remains unchanged.
Recognize vLLM's normalized LB mode and reject conflicting modes or MultiPort in managed roles. Keep one shared HTTP serving port, including an explicitly configured custom port. Validation applies on both create and update; compatibility with previously stored unsupported LB configurations is outside this change.
Group complete examples under
docs/walkthroughs/<module>/. Separate Elastic EP collective growth from fixed External DP serving, share model-prefetch preparation, and preserve captured CR output beside the checks. Move the existing device and KV-cache walkthroughs and update guides, navigation, exports, and skill routes.Which issue(s) this PR links to:
Relates #741
Relates #754
Relates #601
Special notes for your reviewer:
gpustack/runner:cuda13.0-vllm0.29.0ran unchanged. Fixed External TP2/DP2/EP4 passed ordinary and streaming requests directly on both ranks. Four Router requests reached both registered endpoints, with each rank's counter increasing by two.Does this PR introduce a user-facing change?