Repository navigation
Propose user-authored routing for prefill/decode serving - #514
Conversation
7dcf98d to
2381ac8
Compare
6696260 to
e704de1
Compare
8fdeb9f to
c1b0cb8
Compare
|
|
||
| ### The endpoint picker | ||
|
|
||
| `serving.endpointPicker` is optional in both serving modes: |
There was a problem hiding this comment.
Have you thought about how this works on an InferenceCluster that doesn't route with llm-d? I'm thinking of the composed DynamoGraphDeployment in serving-stacks.md. There, Dynamo's operator composes the picker itself, the picker reads DYN_* env vars rather than an EndpointPickerConfig, and P/D runs through Dynamo's own frontend sidecar, not llm-d's.
As written, endpointPicker reads as router-neutral, but its config only means something to llm-d. A P/D deployment also has to write llm-d's sidecar into its decode template even when it omits endpointPicker, so it looks portable but only runs where llm-d routes. And existing P/D deployments have to be rewritten before an upgrade.
One option is to name the router with a discriminator, and make omitting it mean "whatever this cluster routes with":
serving:
mode: PrefillDecode
# Optional, with no default. Omitted, the cluster picks the routing. On today's
# clusters that's what Modelplane composes now, sidecar and NIXL env included,
# so the Kimi-K2 recipe works unchanged.
router: EndpointPicker
endpointPicker:
image: ghcr.io/llm-d/llm-d-router-endpoint-picker:v0.9.0
config: {...}With router set, the author writes the routing, including the sidecar in the decode template as the doc has it now, and the scheduler only places the deployment on clusters that can compose that router. A Dynamo variant can come later without breaking anything.
I haven't thought this through properly (for one, the default path keeps the flag peeking you're removing), so please poke holes in it rather than taking it as-is.
There was a problem hiding this comment.
You're right, this was too llm-d focused. The picker config and the sidecar only mean something where llm-d routes, so every P/D deployment would have been pinned to llm-d and needed a rewrite. I've taken your direction: serving.router is optional with no default. Omitted, the cluster routes as today, flag peeking included, so nothing breaks and a future DGD cluster can route the same deployment its own way. router: EndpointPicker is the user-written path the doc already had. I've also added placement by the routers a cluster advertises, and the resolved router in replica status. We can keep the flag peeking on the default path for now and revisit it separately.
There was a problem hiding this comment.
Thinking about this more, I wonder whether PrefillDecode should have a default at all. What if it required router and the picker config, and only Unified kept a default?
My reasoning is that a wrong default under Unified costs some load-balancing quality, but under PrefillDecode it gives a pair that reports Ready and rejects requests, like #502. A default that suits both vLLM and SGLang, such as always-disagg-pd-decider, looks tempting, but it has Modelplane asserting which engines its config works for again, and I'd expect the next engine that doesn't fit to fail the same way.
serving:
mode: PrefillDecode
router: EndpointPicker # required under PrefillDecode?
endpointPicker:
image: ghcr.io/llm-d/llm-d-router-endpoint-picker:v0.9.0
config: {...} # required under PrefillDecode?If that holds up, it might also remove the catch from my comment above. With no P/D default, Modelplane's P/D code would shrink to wiring, and the vLLM-specific injection and flag reading would go. Existing P/D deployments like the Kimi-K2 recipe would have to add their routing, which seems OK to me before v1.
I haven't thought this through properly either, so I'm curious whether it matches what you've seen.
There was a problem hiding this comment.
Agreed, no default for P/D is better, and it matches what I found. A P/D deployment's engines are already specific to one serving stack (vllm serve behind llm-d's sidecar vs. Dynamo's wrapper), so a default bought no portability. It only hid a vLLM-with-NIXL assumption, and I'd ended up adding a check that warned when the engines didn't match it, which is exactly Modelplane asserting which engines its config works for.
So PrefillDecode now requires router: EndpointPicker and endpointPicker.config, and only Unified keeps the cluster's routing. The P/D code drops the sidecar and NIXL injection, the --port peek and that check, and the port rule loses its exceptions. The Kimi-K2 recipe writes its own routing, and the docs publish the default P/D config as a vLLM starting point to copy.
The design's summary, proposal sketch, and ownership table disagreed with each other and with the background on what Modelplane reads, what a vLLM decode pod needs, and when the user writes the engine's port. They now agree, matching the design branch for modelplaneai#514. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
36ad528 to
0986e1d
Compare
negz
left a comment
There was a problem hiding this comment.
The design looks good to me apart from the picker command line. I lean toward the pod template, but I'm happy to approve as is if you think it doesn't hold up. The other comments are small and shouldn't block.
| - A `Unified` deployment that omits `router` gets Modelplane's picker and | ||
| default configuration. A deployment that sets `router` writes its own. | ||
|
|
||
| Modelplane runs the image with the command line of llm-d's router, which the |
There was a problem hiding this comment.
I wonder whether Modelplane should write this command line at all. It's llm-d's spelling, so router: EndpointPicker only fits pickers that accept it. As far as I can tell that's only llm-d's. Dynamo's picker reads DYN_* env vars, and the only picker in the Inference Extension's 1.6 releases is the lightweight one, where I couldn't find --config-file.
What if we treated the picker the way the base design treats the engine? Modelplane passes what only it knows, such as the pool's name and where it mounts the config, as MODELPLANE_ env vars. The user writes the args, and the fixed contract is just the ports. Shaping it as the same curated pod template subset the engines use means later fields like resources land in both places:
serving:
mode: PrefillDecode
router: EndpointPicker
endpointPicker:
template:
spec:
containers:
- name: picker
image: ghcr.io/llm-d/llm-d-router-endpoint-picker:v0.9.0
args:
- --pool-name=$(MODELPLANE_POOL_NAME)
- --pool-namespace=$(MODELPLANE_POOL_NAMESPACE)
- --pool-group=inference.networking.k8s.io
- --config-file=$(MODELPLANE_PICKER_CONFIG)
- --grpc-port=9002
- --grpc-health-port=9003
- --metrics-port=9090
- --metrics-endpoint-auth=false
config: {...} # Modelplane mounts it at $(MODELPLANE_PICKER_CONFIG)That would make the name EndpointPicker accurate, since any picker that serves the gateway on 9002 and health on 9003 fits. The alternative is renaming it LLMD to match the contract Modelplane hard-codes today.
Curious whether this matches what you saw building #529.
There was a problem hiding this comment.
It does match. Building #529, the command line was the part that kept changing with llm-d: --grpc-health-port arrived in v0.9.0, and --metrics-endpoint-auth=false only showed up when every scrape failed. You're right about the Inference Extension too: its 1.6 picker, lwepp, has no --config-file or --metrics-endpoint-auth, so it would exit on the command line we write.
So the design now takes your shape. endpointPicker.template is a curated pod template with one container, epp, whose image and args the user writes. Modelplane sets MODELPLANE_POOL_NAME, MODELPLANE_POOL_NAMESPACE and, with a config, MODELPLANE_PICKER_CONFIG. The fixed contract is the ports: 9002, 9003 for the readiness probe, and 9090 for metrics. config is optional, since lwepp takes none. A Unified deployment without router still gets llm-d's picker, now written the same way. The old shape and router: LLMD are under Alternatives. #529 implements it, with resources from the template.
|
|
||
| This design doesn't cover: | ||
|
|
||
| - Detecting a `PrefillDecode` pair that reports Ready but doesn't |
There was a problem hiding this comment.
Could we file the issue for this before merging, so the #TBD points somewhere?
|
|
||
| A container that doesn't serve, such as a log shipper, doesn't declare a port, | ||
| and the engine doesn't either. In a serving pod, a `Standalone` pod or a gang's | ||
| leader, CEL checks that: |
There was a problem hiding this comment.
Nit, not blocking: since aligning with #529, some sections read more like a spec of the code than a design. The CEL rules here, rendering the config as JSON with integers, and the labels the upgrade drops are examples. I'd lean toward keeping the rules a reader needs to write a deployment, and leaving the rest to the code and user docs, but I don't feel strongly.
There was a problem hiding this comment.
Agreed, trimmed. The CEL rules are now one sentence on what Modelplane rejects, and the JSON rendering, the dropped labels and the container and port limits are gone. Those stay in the code and the user docs.
The design said the same things in several places: today's routing in the summary, the proposal and its own section, the port-8000 exception twice, modelplaneai#500 as a non-goal and again as a paragraph, and two goals that meant one thing. Small sections for the no-sidecar case, the unchanged vLLM recipe, and two Dynamo future items each stood alone. Each point now appears once. The goals are two, the no-sidecar case sits under the example it explains, the vLLM recipe is one sentence in the SGLang example, and Dynamo is one future improvement. The KServe aside and the detection alternative's extra sentences are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Reviewers had only the design to judge how serving.router reads to a user. The ModelDeployment guide also never said what the default prefill/decode routing needs from the engines, such as a decode --port other than 8000. The guide's disaggregated serving section now lists what the cluster's routing sets and what the engines must set, and points to a new Configure Routing page. That page covers router: EndpointPicker: what the user writes, the picker, the sidecar and what it must match, the port rules, and a full SGLang example. Both describe the proposed API and land for real with the implementation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Four CEL rules governed ports: only the engine could name a port http, the engine had to declare http whenever the pod had another container, and two rules tied 8000 to the engine's port. The first leaked the collector's scrape selector into the API. The second made a log shipper force an http declaration on the engine, and made a worker with a helper declare a port nothing uses. The rule is now that exactly one container serves 8000. If another container declares 8000, the engine declares its http port elsewhere; if the engine moves off 8000, another container declares it. An engine alone, or beside a helper that doesn't serve, declares nothing. The engine's own port is still needed, because llm-d's sidecar answers /health itself (pkg/sidecar/proxy/proxy.go in llm-d-router v0.9.0), so a probe on 8000 would pass before the engine loaded. The design and the draft user docs say so, and sixteen sample replicas behave as intended against a kind API server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The alternatives ran longer than the reasons they were rejected needed. Each is now two sentences: what it is and why it lost. The llm-d router value is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A KV transfer format field and flag detection each need a Modelplane release per format, which the first goal rules out, and always user-written routing would rewrite every existing PrefillDecode deployment, which the second rules out. Only the sidecar field, which meets both goals, stays as an alternative. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A bulleted list of problems read as a list where the argument wanted a paragraph, one sentence broke an inline code span across lines, another said "today's" twice, and a future improvement repeated a fact the proposal already states. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A read of the user docs found the design contradicting itself. The default routing read the decode engine's port from --port and rejected a declared http port, while the user-written path required one. An SGLang deployment that forgot router failed silently. The doc promised a Dynamo cluster could route a default deployment unchanged, which needs an unmerged Dynamo proposal (dynamo#10835). The picker contract was "flags such as", and the Dynamo future improvement relied on an env field that couldn't work: Dynamo 1.5's own picker takes no flags. The 8000 rule moves to the spec level, where it counts the cluster's injected sidecar, so a default-routed decode engine can declare its http port too, and can't declare 8000 beside the sidecar. On the default path only, a decode engine whose args name another format gets a warning to set router. InferenceCluster gains status.routers for placement, and the replica-level router status is dropped until a cluster's default can differ. The doc states the exact picker command line and labels, the all-or-nothing trade-off, and where the sidecar goes in a gang. The draft user docs gain a terms table, the SGLang-without-router warning, the exact default environment, the published default picker configurations, and a disaggregated example tab. Twenty sample replicas behave as intended against a kind API server, and all three CRDs pass a server-side dry run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
…fied A prefill/decode default reports Ready and rejects requests when it doesn't fit the engines, as modelplaneai#502 did with SGLang, and a default that suited more engines would still have Modelplane assert which engines its configuration works for. A P/D deployment's engines are already specific to one serving stack, so the default bought no portability either. PrefillDecode now requires serving.router: EndpointPicker and serving.endpointPicker.config, and only Unified keeps the cluster's routing. That removes the default-routed decode exemptions from CEL and the default-path warning, and makes the port rule one rule. A review also found CEL let two containers declare 8000 and applied the 8000 rule to workers; at most one container may now declare 8000, and Worker members are exempt. The design and the draft user docs say P/D writes its routing, place the SGLang engine-type label in either mode, require a container in front of the engine to pass /metrics through, add the Mooncake specifics, and align the llm-d guide to v0.9.0. Twelve sample replicas behave as intended against a kind API server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The design told existing PrefillDecode deployments to add their routing before upgrading, which the old API can't hold. It also said a worker's ports aren't checked when only the 8000 requirement is waived, and filed reasoning under what Modelplane composes today. The draft user docs gave a PrefillDecode snippet the new CEL rejects, said Modelplane fronts the pair with its own routing, and didn't link a whole example. The endpointPicker description contradicted the PrefillDecode rule. The design now calls the change breaking: after the upgrade a replica without its routing stops reconciling and leaves its running pods and router in place until the ModelDeployment adds router, the picker config and the sidecar. The P/D default reasoning gets its own background section. The docs' snippet sets router and endpointPicker, the routing page links the Kimi-K2 recipe and says the prefill pod needs no sidecar or port, and code fences no longer run into text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The docs said Modelplane rejects a PrefillDecode deployment without its sidecar, which nothing checks, so a pair could still report Ready and serve decode alone. The published prefill/decode starting configuration kept disagg-headers-handler, deprecated in llm-d-router v0.9.0 and removed in v0.11.0. A picker image set without a config ran Modelplane's default configuration on a different picker release. Nothing told today's Kimi-K2 users what happens on upgrade, and status.routers was described as placing deployments before anything reads it. The design and docs now say Modelplane can't check the sidecar, and name the gap as a non-goal with the picker metric a check could read. image and config are set together in CEL. The starting configuration drops the deprecated plugin and can be copied. A routing page section covers upgrading, with the replica's error and what to add, and the docs give the Mooncake and SGLang bootstrap ports, the Unified block size default, the labels Modelplane sets, and the SGLang engine-type label in either mode. The design says compose-inference-cluster sets status.routers from the stack, that Envoy AI Gateway fails closed without a picker, why the SGLang example uses blockSizeTokens: 64, and that sidecars can't mount certificates yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The design still said only config was required under PrefillDecode, though image and config now come together, left "injects neither" without an object, implied every Unified deployment labels its engine type, and filed the labels Modelplane sets under Unified when they apply in both modes. The docs said Modelplane rejects a pod rather than the ModelDeployment, repeated the no-sidecar consequence in the overview, and broke a link across lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The goal stated the engine-type label in the abstract. It now says a Unified deployment at most labels its engine, with SGLang's label as the example. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The Unified goal named the engine-type label without the reason: the cluster's picker reads each engine's own metric names, and picks them by that label. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
modelplaneai#179 asks for a user-set picker configuration and image, not for removing the block-size read. The design now says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The disagg-profile-handler counts its decisions in llm_d_epp_disagg_decision_total. llm_d_epp_pd_decision_total comes from the older pd-profile-handler. Checked against a live llm-d-router v0.9.0 picker on GKE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The API change, the regenerated models and the draft user docs move to the implementation PR, beside the code and tests that use them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A running replica keeps its old picker, which serves the old configuration, and the replica still reports Ready. Seen on GKE with a plugin type llm-d-router v0.9.0 doesn't register. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A decode pod is Ready once its engine is, whether or not the sidecar listens. List a readinessProbe beside resources and volume mounts as a field the containers beside the engine could gain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
- router: EndpointPicker requires endpointPicker with both image and config, so there is one way to get the default and one to replace it. - Containers take resources, and a container beside the engine can set a readinessProbe; the examples probe the sidecar's /health. - status.routers arrives with stack-aware placement (modelplaneai#418), not before. - Modelplane sets only llm-d.ai/role on PrefillDecode pods, passes --grpc-health-port, warns when the decode pod has no container beside the engine, and reports a replica not Ready once a rejected picker configuration's rollout passes its deadline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
- List every port rule CEL checks, including an unnamed 8000 on the engine and unique port names, and say Workers skip them. - Say the upgrade rolls every picker, and that PrefillDecode pods lose the app and llm-d.ai/inference-serving labels. - The NIXL environment went only to PrefillDecode serving pods, and the role label goes only on serving pods. - The picker raises blockSizeTokens below 64 rather than rejecting it, and Modelplane renders the configuration as JSON. - Name everything Modelplane adds to the engine, the collector's container match, and the probe handler rule. - Mark endpointPicker, not config, as required with router, and link vLLM v0.23.0 docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The design had Modelplane run the picker image with llm-d's command line, which only llm-d's picker accepts. The picker is now a pod template whose command line the user writes, with Modelplane passing the pool and the configuration's path as environment and fixing only the ports. The hard-coded command line and a router named for llm-d are under Alternatives. Sections that described the code rather than the design are trimmed, and the follow-up for a pair that reports Ready but doesn't disaggregate is modelplaneai#552. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Dennis Ramdass <dennis@upbound.io>
9d1edcb to
ed19eaf
Compare
| - --grpc-port=9002 | ||
| - --grpc-health-port=9003 | ||
| - --metrics-port=9090 |
There was a problem hiding this comment.
I'd make these ports env vars too, even though they're pretty static. It helps clarify they're part of the contract and that Modelplane has expectations about what they are. (It also lets us change port for metrics if we want to in future.)
There was a problem hiding this comment.
|
|
||
| ### Containers beside the engine | ||
|
|
||
| This design adopts #108, in either serving mode. A member's template can list |
There was a problem hiding this comment.
Nit: The design would be easier to read if these issue refs were clickable links.
There was a problem hiding this comment.
Done in #529 too: every issue reference in the design is a link now.
Description of your changes
Modelplane writes the prefill/decode router itself, and it writes only one: the llm-d sidecar with
--kv-connector=nixlv2,VLLM_NIXL_*environment on each engine of the pair, and a picker configuration whose decider skips prefill for short prompts. An SGLang prefill/decode deployment gets that router anyway. It reports Ready, and its decode engine rejects every request (#502):Each new KV connector would need a Modelplane release, and each llm-d router release can break the router Modelplane writes.
This PR adds
design/user-authored-routing.md. It proposes that aPrefillDecodedeployment writes its own router: the endpoint picker's container, command line and configuration, and the llm-d sidecar as a container besideenginein the decode member (#108). Modelplane passes the picker what only it knows as environment, and fixes only its ports, so any picker that speaks the protocol fits, not just llm-d's.Unifiedkeeps a default. Settingrouterreplaces it with the user's picker. Prefill/decode doesn't get one. A wrongUnifieddefault costs some load-balancing quality, but a wrong prefill/decode default gives a pair that reports Ready and rejects requests, and a prefill/decode deployment's engines are tied to one serving stack anyway.This breaks existing
PrefillDecodedeployments. Their replicas stop reconciling and keep serving as they are, until the deployment adds its routing.The calls that need review are no default for prefill/decode, the picker as a pod template whose command line the user writes, and the breaking change.
#529 builds it, with the API change, tests and user docs. It ran on GKE with SGLang and vLLM prefill/decode pairs and a vLLM decode gang, and each one disaggregated.
Refs #502, #108, #179, #418, #500
I have:
Runnix flake check(or./nix.sh flake check) and made sure it passes.Added or updated tests covering any composition function changes.git commit -s.🤖 Generated with Claude Code