feat: pd disaggregated - #1410
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive support for Prefill/Decode (PD) disaggregation, cluster topology management, RDMA/InfiniBand worker configuration, and enhanced benchmark metrics (including Inter-Token Latency and target modes) across multiple localized files. The review feedback highlights a potential runtime crash in the role GPU selector mapping when gpu_selector is undefined, a timer bug in the topology location cell where failed save timeouts are not properly cleared, and several untranslated localization keys left in English across the Japanese, Russian, and Turkish locale files.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved target selection, topology mapping, legacy conversion, accessibility, and security issues can produce incorrect behavior or inaccessible state.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (9)
What changed in this PR
Adds PD-disaggregated deployment support across model management, topology configuration, worker setup, and benchmarking.
Changes:
- Adds PD role configuration, status, restart, scaling, and topology workflows.
- Adds RDMA worker setup and bulk worker location/label management.
- Expands benchmarking with route targets, tail-latency metrics, and deployment-member reporting.
| File | Description |
|---|---|
src/pages/resources/hooks/use-worker-columns.tsx |
Adds worker location column. |
src/pages/resources/config/types.ts |
Adds topology facts. |
src/pages/resources/config/gpu-driver.ts |
Adds RDMA Docker arguments. |
src/pages/resources/components/worker-right-actions.tsx |
Adds bulk label/location actions. |
src/pages/resources/components/update-labels.tsx |
Supports batch label editing. |
src/pages/llmodels/instance-view/index.tsx |
Handles instance creation events. |
src/pages/llmodels/hooks/use-run-benchmark.ts |
Adds model-level benchmark targeting. |
src/pages/llmodels/hooks/use-restart-model.ts |
Adds deployment restart flow. |
src/pages/llmodels/hooks/use-form-initial-values.ts |
Hydrates worker vendors and roles. |
src/pages/llmodels/forms/vgpu-type.tsx |
Supports role-prefixed vGPU fields. |
src/pages/llmodels/forms/roles/role-kv-cache.tsx |
Adds role-level KV cache form. |
src/pages/llmodels/forms/performance.tsx |
Hides model performance settings under PD. |
src/pages/llmodels/forms/custom-backend.tsx |
Supports role-prefixed custom backends. |
src/pages/llmodels/forms/basic.tsx |
Moves replica control into PD roles. |
src/pages/llmodels/forms/backend.tsx |
Supports role-level backend overrides. |
src/pages/llmodels/forms/backend-parameters-list.tsx |
Supports role-level parameters. |
src/pages/llmodels/forms/advance-config.tsx |
Adjusts advanced settings for PD. |
src/pages/llmodels/components/table-list.tsx |
Adds restart, benchmark, and role scaling actions. |
src/pages/llmodels/components/pd/role-status.ts |
Adds role-status calculations. |
src/pages/llmodels/components/pd/role-status-detail.tsx |
Renders role readiness details. |
src/pages/llmodels/components/pd/role-group-header.tsx |
Adds collapsible role headings. |
src/pages/llmodels/components/pd/pd-markers.tsx |
Adds stale/degradation markers. |
src/pages/llmodels/components/instance/instances.tsx |
Groups instances by PD role. |
src/pages/llmodels/components/instance-cells/name-cell.tsx |
Shows role and stale annotations. |
src/pages/llmodels/components/instance-cells/kv-cache-degraded-cell.tsx |
Refines cache degradation tooltip. |
src/pages/llmodels/components/instance-cells/instance-status-cell.tsx |
Adds draining status. |
src/pages/llmodels/components/instance-cells/actions-cell.tsx |
Restricts group-member benchmarking. |
src/pages/llmodels/components/deployment/update-modal.tsx |
Improves loading and role GPU hydration. |
src/pages/llmodels/components/deployment/deploy-modal.tsx |
Shows validation-time loading. |
src/pages/llmodels/apis/index.ts |
Adds PD-related APIs. |
src/pages/cluster-management/hooks/use-cluster-columns.tsx |
Adds topology action. |
src/pages/cluster-management/config/index.ts |
Adds topology labels and hints. |
src/pages/cluster-management/components/topology/spec.ts |
Loads and saves topology specifications. |
src/pages/cluster-management/components/topology/onboarding.tsx |
Adds topology onboarding. |
src/pages/cluster-management/components/topology/location.ts |
Adds location resolution helpers. |
src/pages/cluster-management/components/topology/hooks/use-preview.ts |
Adds debounced topology previews. |
src/pages/cluster-management/components/topology/hooks/use-column-prefs.ts |
Persists topology columns. |
src/pages/cluster-management/components/topology/column-settings.tsx |
Adds topology column controls. |
src/pages/cluster-management/components/topology/advanced/rename-layer.tsx |
Adds layer renaming. |
src/pages/cluster-management/components/topology/advanced/key-editor.tsx |
Adds label-key editing. |
src/pages/cluster-management/components/topology/advanced/draft.ts |
Adds topology draft transforms. |
src/pages/cluster-management/components/add-worker/specify-arguments.tsx |
Adds RDMA selection. |
src/pages/cluster-management/components/add-worker/docker-run-command.tsx |
Passes RDMA configuration. |
src/pages/cluster-management/components/add-worker/config.ts |
Types RDMA settings. |
src/pages/cluster-management/components/add-worker-command.tsx |
Generates RDMA commands. |
src/pages/cluster-management/clusters.tsx |
Hosts the topology drawer. |
src/pages/cluster-management/apis/index.ts |
Adds topology endpoints. |
src/pages/benchmark/index.tsx |
Updates benchmark filtering. |
src/pages/benchmark/hooks/use-column-settings.tsx |
Adds target and latency columns. |
src/pages/benchmark/forms/random-settings.tsx |
Uses percentage-based limits. |
src/pages/benchmark/forms/index.tsx |
Converts percentage fields. |
src/pages/benchmark/filters/index.tsx |
Adds target-mode filtering. |
src/pages/benchmark/details.tsx |
Adds monitoring link. |
src/pages/benchmark/config/types.ts |
Extends benchmark metric types. |
src/pages/benchmark/config/index.ts |
Defines benchmark target modes. |
src/pages/benchmark/config/detail-types.ts |
Extends benchmark snapshots. |
src/pages/benchmark/components/summary/percentile-result.tsx |
Adds ITL percentiles. |
src/pages/benchmark/components/summary/overview.tsx |
Adds tail-latency columns. |
src/pages/benchmark/components/summary/metrics.ts |
Computes ITL metrics. |
src/pages/benchmark/components/summary/metrics-result.tsx |
Displays metric fallbacks and ITL. |
src/pages/benchmark/components/summary/instance.tsx |
Describes deployment targets. |
src/pages/benchmark/components/summary/deployment-members.tsx |
Lists deployment members. |
src/pages/benchmark/components/summary/chart-specs.ts |
Adds ITL charts. |
src/pages/benchmark/components/summary/best-points.tsx |
Uses p99 latency summaries. |
src/pages/benchmark/components/summary/benchmark.tsx |
Shows percentages and stage limits. |
src/pages/benchmark/components/left-actions.tsx |
Adds multi-name and target filters. |
src/pages/benchmark/components/environment/use-worker-columns.tsx |
Shows hosted members. |
src/pages/benchmark/components/environment/index.tsx |
Aggregates hardware across members. |
src/pages/benchmark/components/dashboard-link.tsx |
Links benchmark monitoring. |
src/pages/_components/source-config/slot-form.tsx |
Resets editor viewport after save. |
src/locales/zh-CN/resources.ts |
Adds Chinese resource strings. |
src/locales/zh-CN/benchmark.ts |
Adds Chinese benchmark strings. |
src/locales/tr-TR/resources.ts |
Adds Turkish resource strings. |
src/locales/ru-RU/resources.ts |
Adds Russian resource strings. |
src/locales/ja-JP/resources.ts |
Adds Japanese resource strings. |
src/locales/en-US/resources.ts |
Adds English resource strings. |
src/locales/en-US/benchmark.ts |
Adds English benchmark strings. |
src/atoms/models.ts |
Adds worker vendor metadata. |
.claude/skills/design-review/SKILL.md |
Extends expanded-row review guidance. |
…r saving Re-seeding the editor with what was saved leaves the viewport where the last edit left it. After pasting a document, that is its end — and an editor parked past the last line reads as an empty box.
The chunked-list hook defaults to ['UPDATE', 'DELETE'], which silently loses rows here: an instance the scheduler just created arrives as CREATE, whose branch the default skips, and the UPDATE that follows carries an id the list has never seen — inserted only when 'INSERT' is among the events too. With neither, a new instance reaches the list by no path at all.
An NVIDIA worker on a host with InfiniBand/RoCE NICs needs the IB devices and the privileges RDMA asks for, and mirrored deployment passes both on to every runner container it starts — which is how an inference backend reaches the fabric for cross-node KV cache transfer. The toggle is offered only where the command template consumes it (NVIDIA), and probes the active IB ports at run time rather than pinning an interface name.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The generated RDMA command is invalid and 100% warmup/cooldown values are misencoded as request counts.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (4)
Resolved since last review (9)
A cluster gains a topology declaration — an ordered chain of location layers (region, zone, room, row, rack, access switch, accelerator domain, host), each reading its value from a worker label key. The drawer paints the fleet against that mapping as a table or a tree, previews an unsaved mapping before it is kept, and fills locations in place; a selection can be labelled forty machines at a time, which is how a fleet is actually maintained. Values the runtime reads off the hardware (`topology_facts`, today the NVIDIA NVLink clique) show as discovered and are overridable by hand. Locations only affect later scheduling — nothing running is moved. Carries the `gather` field on the model type, which the layer view reads to say which models would lose a constraint if a layer were removed.
A deployment can now be declared as a group of roles — prefill, decode and a router — instead of a single uniform pool. The form derives the transport scheme from the cluster's accelerator vendor and the engine's built-in recipe, falling back to `custom` where the user owns every connection parameter, and each role inherits the model's settings unless it overrides one of four groups (backend, parameters, scheduling, cache). Operating the group is the other half: - `state` replaces `ready_replicas > 0` as the servability gate. A 3P1D whose router is down is four running instances and zero service. - The replica column's denominator is the sum of the roles' declared counts; under PD `replicas` is a 0/1 deployment switch and would render "5 / 1". - Degradations are orthogonal to `state` — serving, but worse than asked for: KV not moving at all, pairing that never formed, a locality layer unmet, an engine below the recipe's floor, a pair nothing verified. - A member picked by scale-down shows as draining rather than vanishing silently after its window. - The instance list groups by role, and the expansion is headings and members only. Also carries the deploy-form half of topology affinity (`gather`), and a `vendors` field on the cluster atom, which is the only place a form can learn which accelerators a cluster actually has.
All three are stored in guidellm's own scalar convention, where a value below 1 is a fraction. Sending the typed number straight through was a footgun for warmup and cooldown, which guidellm reads as an absolute REQUEST COUNT at 1 and above: a 10 meant as 10% landed as an absolute 10. Max error rate had the mirror problem — the only field in the form asking for a raw fraction, reading as inconsistent beside its neighbours. The form now owns the percent unit and converts at the boundary, in both directions; a stored warmup of 1 or more predates the change and is shown as the count it is rather than multiplied into a nonsensical 1000%. Each field gained the tooltip that says what it trims (a share of the stage's requests, not of its seconds). The configuration panel also tabulates per-stage caps where the stages declare them — a real sweep runs 300 requests at C=1 but 5120 at C=512, and the bare rate list showed none of it.
A run can now be aimed at either an instance or a route. `instance` is what every run was before the choice existed — the load goes straight at one engine, on its own port, with nothing else in the path. `route` drives the entrance clients actually call, which is the only correct target for a disaggregated group: its members cannot answer a request on their own, and a run aimed at a prefill returns 200 after a single token while a decode runs without the prefix its KV was meant to carry. Neither errors, and the report describes a request path no user request takes. A group therefore hands over its model rather than a member, and the report names the member whose engine configuration it describes instead of the router, which runs no model. The environment snapshot covers every member the run touched — a group's cards are spread across its prefill and decode members, and the endpoint alone would report the run on zero cards.
Opens the dashboard for the run over the interval it actually ran, so the numbers in the report can be read against what the cluster was doing at the time. A disaggregated group goes to the PD dashboard, which is where its transfer and pairing series live.
TPOT reads guidellm's `inter_token_latency_mean` — one value per REQUEST, the decode-only per-token time. Measured ITL is a genuinely different reading: the gaps BETWEEN consecutive streamed outputs, one sample per gap, pooled across requests. The difference only shows in the tail, and that is the whole point. A request that streams 500 tokens at 10ms and stalls once for 800ms has a TPOT of 11.6ms — the stall divided away by the other 499 gaps, i.e. invisible — while its gaps put an 800ms sample where a reader can see it. Both metrics now carry their p95 and p99 next to their mean, and the summary tables lead with p99 rather than the mean: that is the column an SLO is written against, and a mean TTFT hides exactly the queueing a sweep exists to find. The means stay in the stage detail. The column-settings panel groups means and tails apart so a metric and its percentiles never straddle rows. Older runs have null in the new columns, which means NOT MEASURED — never "the gaps were 0 ms" — so the metrics card falls back to the quantile in the dump where the flat column is absent.
Three references to a server that never answers them, none of them reachable
from the UI today:
- `queryModelPDMetrics` and `estimateKVTransferBudget` call
`GET /models/{id}/pd-metrics` and `POST /models/kv-transfer-budget`. The
`GroupSummary` panel that consumed them was removed with its hooks; the two
functions, their response types and the five interfaces beneath them stayed,
along with a comment promising the endpoints still answer. They do not — the
server has no route by either name. The next reader to wire them up would
have found that out from a 404.
- The `pd_ineffective` degradation is not in the server's
`DegradationReasonEnum`, so the badge and its five translations could never
render. Its neighbour `bandwidth_degraded`, named in the same comment, does
not exist either. Reinstating the marker means reinstating the metrics
endpoint it reads, which is a feature rather than a fix; until then a code
that cannot arrive is worse than no code, because it reads as covered.
- `createBenchmarkResult` posts to `/benchmarks/{id}/result`, singular, where
the server serves `/results`. Unreferenced, and predates this work.
The server has recorded `spec_digest` on every snapshot — a content hash over the model's definitional fields — and nothing displayed it, so the question it answers could not be asked: are two reports about the same configuration, or did the deployment change between them? Names do not settle that; a model can be edited in place and keep its name. Shown truncated beside the other snapshot facts, with the full value on hover. Nobody reads a hash, but comparing the first few characters of two is exactly the use, and the full one is there for when the answer has to be exact.
It said the element "bridges antd's static-looking APIs (modal.confirm,
message) into this ConfigProvider". It does not: `App.useApp()` is what hands a
component the wired versions, and a module-level `import { message } from
'antd'` keeps rendering in antd's own detached root regardless of what is
mounted here.
The distinction is measurable rather than cosmetic. `global.less` colours a
toast's text with a project variable declared on `html[data-theme='realDark']`
— inherited even by a detached container — over a background from antd's
cssVar, which is scoped to the ConfigProvider container a static API never
enters. In dark mode the pair lands around 2.0–2.3:1, against the 4.5:1 those
same styles carry measurements for.
Collaborator
|
LGTM |
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.
No description provided.