-
Notifications
You must be signed in to change notification settings - Fork 694
docs: add disagg example + explanation #2086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new, detailed README file was added under the examples directory to demonstrate and document Dynamo's Prefill/Decode Disaggregated Serving architecture for large language model inference. The documentation explains system components, prerequisites, operational flow, deployment instructions, and architectural diagrams, but does not introduce or modify any code. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
examples/basics/disaggregated_serving/README.md (5)
40-54: Mermaid front-matter prevents GitHub renderingGitHub strips diagrams when a mermaid code block begins with YAML front-matter (
---).
Remove lines 41-43 or move thetitle:line inside a%%{init:...}%%comment so the diagram renders in the GitHub UI.-```mermaid ---- -title: Disaggregated Request Flow ---- +```mermaid +%%{init: {"theme": "default"}}%%
65-67: Explicitly mark the worker role for clarityThe decode worker is launched with the default flags. Add
--role decode(or the documented equivalent) so readers don’t wonder whether the same command can accidentally start a prefill worker.CUDA_VISIBLE_DEVICES=0 python -m dynamo.vllm \ --role decode \ --model Qwen/Qwen3-0.6B
80-82: Flag consistency:--is-prefill-workervs--role prefillIf the CLI recently standardised on
--role, prefer the same flag here; otherwise document both forms to avoid confusion.CUDA_VISIBLE_DEVICES=1 python -m dynamo.vllm \ --role prefill \ --model Qwen/Qwen3-0.6B
170-175: Add reference link to NIXLReaders unfamiliar with NVIDIA NIXL have no easy way to learn more.
Add an external link to the official docs or white-paper after the first mention to improve discoverability.The architecture relies on NVIDIA's [NIXL](https://developer.nvidia.com/...link...) ...
213-216: Minor wording: avoid double “workers”“workers distributed across multiple machines for larger scale”
Consider rephrasing slightly for readability:
“workers distributed across multiple machines to scale beyond a single node”
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/basics/disaggregated_serving/README.md(1 hunks)
🔇 Additional comments (2)
examples/basics/disaggregated_serving/README.md (2)
30-32: Confirm the Compose file path
deploy/metrics/docker-compose.ymllooks like an infra-metrics stack rather than the etcd + NATS stack referenced in the text.
Double-check that the Compose file actually starts both etcd and NATS (and nothing extra) or point to the correct compose file (e.g.deploy/core/docker-compose.yml).
Otherwise readers may start the wrong services.
36-39: The existing relative links are correct – no change needed
The links use../../../components/...fromexamples/basics/disaggregated_serving/README.md, which resolves tocomponents/...at the repo root. The reviewer’s suggested../../../../overshoots and would break the paths.
Please disregard the suggested update.Likely an incorrect or invalid review comment.
612b45a to
fae09c0
Compare
whoisj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved w/ a comment\question.
Add an example of vllm disaggregated serving with some explanation of what's going on
Summary by CodeRabbit