feat: add P/D disaggregation output to llm-d generator - #283
Open
amito wants to merge 1 commit into
Open
Conversation
amito
force-pushed
the
feat/pd-disaggregation
branch
5 times, most recently
from
July 19, 2026 11:50
2753ae4 to
73b361f
Compare
amito
force-pushed
the
feat/pd-disaggregation
branch
4 times, most recently
from
July 22, 2026 05:41
4103d42 to
cce5a40
Compare
amito
marked this pull request as ready for review
July 29, 2026 13:02
jgchn
reviewed
Aug 3, 2026
| pd_enabled = st.checkbox( | ||
| "Enable P/D Disaggregation", | ||
| value=st.session_state.get("pd_enabled", False), | ||
| key="pd_enabled", |
Collaborator
There was a problem hiding this comment.
Could be a future improvement, but the "Deploy to Kubernetes" path doesn't forward the new params like pd_enabled,prefill_replicas and decode_replicas.
Comment on lines
+84
to
+86
| pd_enabled = st.checkbox( | ||
| "Enable P/D Disaggregation", | ||
| value=st.session_state.get("pd_enabled", False), |
Collaborator
There was a problem hiding this comment.
I think we need to clear the stale YAMLs after user checks this box?
Collaborator
Author
There was a problem hiding this comment.
Agreed, fixing this, thanks.
Comment on lines
+86
to
+91
| if prefill_replicas < 1: | ||
| msg = f"prefill_replicas must be >= 1, got {prefill_replicas}" | ||
| raise ValueError(msg) | ||
| if decode_replicas < 1: | ||
| msg = f"decode_replicas must be >= 1, got {decode_replicas}" | ||
| raise ValueError(msg) |
Collaborator
There was a problem hiding this comment.
Not a bug but the generation checks for two different ranges. The Pydantic model in configuration.py checks for Field(1, ge=1, le=32) but here, the generation checks for just <1. Perhaps we should remain consistent?
Collaborator
Author
There was a problem hiding this comment.
I agree. Fixing this for consistency. Thanks
amito
force-pushed
the
feat/pd-disaggregation
branch
from
August 20, 2026 06:34
cce5a40 to
45abee6
Compare
Add prefill/decode disaggregation as an output option to the llm-d deployment generator. When pd_enabled=True, the generator produces separate Kustomize patches for prefill and decode deployments instead of the single patch-vllm.yaml. All three patch variants (vllm, prefill, decode) use a single unified template (patch-modelserver.yaml.j2) rendered with different context (deployment_name, replica_count, extra_args) to avoid maintaining near-identical templates that would drift. If the roles diverge significantly, splitting is a one-step refactor. Also adds Field(ge=1) validation on prefill_replicas/decode_replicas in the API schema. Signed-off-by: Amit Oren <amoren@redhat.com>
amito
force-pushed
the
feat/pd-disaggregation
branch
from
August 20, 2026 06:35
45abee6 to
dde775f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds prefill/decode (P/D) disaggregation as an output option to the llm-d deployment generator. When P/D is enabled, the generator produces separate Kustomize patches for prefill and decode deployments instead of the single patch-vllm.yaml, allowing users to configure independent replica counts and vLLM arguments for each role.
When
pd_enabled=False(the default), output is identical to the current behavior introduced in PR #274. No breaking changes.What gets generated when P/D is enabled
--kv-role=kv_producer,--enable-chunked-prefill, and configurableprefill_replicas--kv-role=kv_consumerand configurabledecode_replicasKV transfer uses
nixlv2(NixlConnector) with separate--kv-connectorand--kv-roleargs, matching the llm-d P/D well-lit path.Template consolidation
All three patch templates (patch-vllm, patch-prefill, patch-decode) are consolidated into a single
patch-modelserver.yaml.j2rendered with different context variables (deployment_name,replica_count,extra_args). This avoids maintaining three near-identical templates that would drift independently. Role-specific differences (e.g., kv-connector args, chunked-prefill) are passed asextra_argsfrom the generator code. If the prefill and decode roles diverge significantly in the future (different sidecars, volumes, probes), splitting back into separate templates is a one-step refactor.Changes
generate_all()acceptspd_enabled,prefill_replicas, anddecode_replicasparameters. Renders the unified template with role-specific context. Validates replica counts >= 1 at the generator level (raisesValueError).pd_enabled.pd_enabled,prefill_replicas, anddecode_replicasfields toDeploymentRequestwithField(ge=1, le=32)validation on replica counts.patch_prefill/patch_decodekeys.TestPDDisaggregation(8 tests),TestDeployAPINewParams(4 tests). 31 tests total.Note on
deployment_name: The non-PD patch usesmetadata.name: decode(notvllm) because it is a strategic merge patch targeting the upstream llm-d baseDeployment/decode. A dedicated test locks this down.How Has This Been Tested?
12 new unit tests added, full suite passes (31 tests in test_llmd_generator.py):
Tests cover:
test_pd_disabled_produces_single_patch- Default behavior unchanged:patch_vllmpresent, nopatch_prefill/patch_decode.test_pd_disabled_patch_targets_decode_deployment- Non-PD patch usesmetadata.name: decodeto match the llm-d base Deployment.test_pd_enabled_produces_prefill_and_decode_patches- P/D mode producespatch_prefillandpatch_decode, nopatch_vllm.test_pd_prefill_patch_has_correct_replicas- Prefill patch usesprefill_replicasvalue (non-default) and hasmetadata.name: prefill.test_pd_decode_patch_has_correct_replicas- Decode patch usesdecode_replicasvalue and hasmetadata.name: decode.test_pd_kustomization_references_both_patches- Kustomization patches list contains patch-prefill.yaml and patch-decode.yaml, not patch-vllm.yaml.test_pd_all_outputs_valid_yaml- All generated contents parse as valid YAML.test_rejects_zero_replicas(parametrized) - Generator raisesValueErrorwhenprefill_replicasordecode_replicas< 1.test_deploy_llmd_with_pd_enabled- API integration test: POST withpd_enabled: truereturns200withpatch_prefillandpatch_decode.test_deploy_rejects_zero_replicas(parametrized) - API returns 422 whenprefill_replicasordecode_replicasis 0.test_deploy_rejects_replicas_above_max- API returns 422 when replica count exceeds 32.UI manually tested: selecting llm-d stack and enabling the P/D checkbox shows prefill/decode replica inputs; generated files display correctly with the new file labels.
Merge criteria: