gpustack / gpustack/gpustack-operator

bug: a MultiConnector aborts the engine on Prometheus metrics registration

Open
#454 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area/worker kind/bug
Dominant language
Go
Stars
4
Forks
7
Avg merge
3h 9m
Merged PRs (30d)
213

Description

What happened:

When a ModelDeployment configures both a shared KV cache (spec.kvCache) and disaggregated P/D
roles, this operator renders vLLM a MultiConnector holding MooncakeConnector (direct transfer)
and MooncakeStoreConnector (shared store). On a real cluster run, the prefill role's engine hit:

AssertionError: MooncakeConnector is not contained in the list of registered connectors
with Prometheus metrics support: dict_keys(['MooncakeStoreConnector'])

The API server process aborted and recovered on restart. KV transfer itself works — what fails is
the metrics aggregation step that runs after a transfer has produced statistics.

What you expected to happen:

A deployment that uses both connectors should serve without the engine process aborting.

How to reproduce it (as minimally and precisely as possible):

A ModelDeployment with spec.kvCache set and a prefill/decode role pair, on a vLLM runner, with
enough traffic that the direct-transfer connector actually moves blocks. The abort needs a transfer
to have happened: a deployment that never transfers never reaches it.

How this was found (tick one):

  • Hit it — running the operator, doing something I meant to do
  • Reported — somebody else hit it and told us
  • Worked out — read the code, a specification or a manifest and derived that this must happen

Observed first on a cluster run, then traced to the upstream source to establish why.

Anything else we need to know?:

Root cause: an upstream invariant that two correct implementations break together

vLLM gives a connector two independent optional capabilities. One reports transfer statistics, the
other registers Prometheus metrics. The base class makes the second one optional by returning None:

get_kv_connector_stats build_prom_metrics
MooncakeConnector yes — kv_connector/v1/mooncake/mooncake_connector.py:617 no — inherits the base, and kv_connector/v1/base.py:733-746 returns None
MooncakeStoreConnector yes — kv_connector/v1/mooncake/store/connector.py:386 yes — kv_connector/v1/mooncake/store/connector.py:402

MultiConnector.build_prom_metrics (kv_connector/v1/multi_connector.py:674-700) only puts a child
in its registry when that child returns something other than None. So the registry holds
MooncakeStoreConnector alone — which is exactly what dict_keys(['MooncakeStoreConnector'])
reports. The registry is not failing to populate; it was never going to hold the other one.

MultiKVConnectorPromMetrics.observe (multi_connector.py:124-131) then walks the connectors that
did report statistics and asserts each one is in that registry. MooncakeConnector reports and is
absent, so the assertion fires.

The invariant that breaks: upstream permits a connector to report statistics without providing
Prometheus metrics, and MultiConnector.observe treats "reported statistics" as "must be
registered". Both connectors are individually consistent with the base class. Putting them in one
MultiConnector is what breaks.

It fires when the feature is working

MooncakeConnector only has statistics to report once it has moved KV blocks. A successful direct
transfer is the trigger for this abort, not its opposite.

Version span

Checked by reading the three observables (no build_prom_metrics on the P2P connector, one on the
store connector, the assertion present) out of each tag:

tag affected
v0.25.1 yes
v0.27.1 yes
v0.28.0 yes
v0.29.0 yes
v0.29.1rc0 yes
main (v0.29.1rc0-65) yes

Unchanged across every release checked, including the current tip.

What this operator can and cannot do

The fix belongs upstream and takes one of two forms: give MooncakeConnector a build_prom_metrics
implementation, or make MultiConnector.observe skip a connector that registered none. Neither is in
this repository.

Disabling the metrics is not a free workaround. There is no per-connector switch:
v1/metrics/loggers.py:1058-1061 gates the call only on statistics being present. The adjacent
kv_cache_metrics_enabled flag (:465) governs the KV cache eviction histograms (:913, :1067)
and not this call. The only switch that reaches it is --disable-log-stats, which disables the whole
stat logger — and the managed router reads that same endpoint for its scoring input
(pkg/worker/kvcache/router/metrics.go requires vllm:num_requests_waiting,
vllm:num_requests_running and vllm:kv_cache_usage_perc). Disabling it trades this abort for a
router with no metrics to route on.

So the two options available today are to avoid the shape (do not configure a shared KV cache and
direct transfer together, which is the only shape MultiConnector is rendered for — see
pkg/worker/kvcache/inject/vllm.go:169-191), or to carry the abort.

The intended remedy is a patched runner image: gpustack/runner supports a post operation that
mutates an already released image, so the patch can be applied to the cuda, rocm and cann
backends without waiting for an upstream release.

Not applicable to vLLM-Ascend as written

vllm-ascend removes upstream's MultiConnector from the connector factory and registers its own
AscendMultiConnector
(vllm_ascend/distributed/kv_transfer/__init__.py). The analysis above is of upstream's class and
does not carry over; the Ascend path needs its own reading.

What does NOT close this
  • An upstream fix landing. Users run pinned runner images, so an upstream release does not reach a
    deployment until that image is rebuilt or patched.
  • Documenting the combination. The abort happens in the data plane, after a transfer succeeded;
    a reader who needed the documentation has already passed the point where it would have helped.
  • A deployment that runs without aborting but never transferred anything. The abort needs a transfer
    to have produced statistics, so "it ran and nothing happened" is the state this issue exists to
    distinguish from a fix.

Environment:

  • Kubernetes version (use kubectl version): observed on a managed cluster during P/D verification
  • GPUStack version: n/a
  • GPUStack Operator version: main
  • Cloud provider or hardware configuration: two GPU nodes, TCP data plane
  • OS (e.g: cat /etc/os-release): n/a
  • Kernel (e.g. uname -a): n/a
  • Install tools: Helm chart
  • Accelerator preflight: n/a — the defect is in the engine's metrics path, not in device discovery
  • Others: vLLM runner builds 0.25.1 and 0.27.1

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at pkg/worker/kvcache/inject/vllm.go:169-191 to confirm when the operator renders MultiConnector, then read upstream kv_connector/v1/multi_connector.py and the Mooncake connector files cited in the report. Verify the failure with a transfer that produces statistics; done requires a patched runner image or an upstream-compatible fix that prevents the engine abort while preserving router metrics.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes, prometheus, python
Domain
backend, infrastructure, observability
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.