fix(agentic): give KV cache series a real per-engine identity - #710
Open
cquil11 wants to merge 3 commits into
Open
fix(agentic): give KV cache series a real per-engine identity#710cquil11 wants to merge 3 commits into
cquil11 wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The KV cache utilization chart drew one line per raw metric series rather than one per engine, so DEP points rendered a tangle of duplicate lines with an unreadable legend — the 8-rank DP run at /inference/agentic/439261 drew 32 lines labelled "DP 0 DP 0 DP 0 DP 0 DP 1 ...". Three independent duplications were folded into `kvCacheUsageByEngine`: 1. v12's warmup merge concatenates each engine's warmup and profiling series, so every engine appeared at least twice. Single-engine points also began drawing a spurious two-line "per-engine" overlay. 2. vLLM run with several API-server frontends exposes the *same* engine set on every /metrics endpoint, ~176 ms apart, so 8 DP ranks became 16. 3. Tensor-/pipeline-/expert-parallel ranks each report the one KV pool they share, so a TP8 SGLang worker looked like 8 engines holding identical values. The same fragmentation corrupted the cluster-average line, which grouped on an exact `start_ns`: each tick averaged only the engines that happened to share that nanosecond. On disaggregated runs that alternates between "prefill only" and "decode only" — a full-scale sawtooth, not an average. Series are now grouped by their Prometheus label set (endpoint_url is transport, not identity; intra-engine shard ranks are excluded), and the average is a true mean across logical engines on the union of their scrape instants, each engine holding its last sample only inside its own observed window. CHART_SERIES_VERSION 12 -> 13; run db:backfill-chart-series. Measured on real blobs (roughness = mean |tick delta| / stddev; a clean 1 Hz single-grid row sits near 0.18): point 439261 vllm/mi355x/dsv4 DEP8 c=64 32 -> 8 engines 0.59 -> 0.05 point 439292 dynamo-sglang/h200/glm5.2 64 -> 18 engines 1.00 -> 0.02 point 439312 dynamo-sglang/gb300/dsv4 10 -> 5 engines 1.29 -> 0.07 point 436497 vllm/b200/dsv4 c=8 16 -> 8 engines unchanged point 437312 dynamo-vllm/b200/kimik3 2 -> 0 engines unchanged 229 of the 436 stored rows carrying a per-engine breakdown were affected. Single-endpoint rows come out byte-identical; disaggregated rows shift level because the mean now weights every engine once instead of over-weighting whichever subset shared a timestamp. Sum-combined metrics (queue depth, prefill/decode TPS) still group on exact start_ns and are untouched here: their point count feeds running-sum cumulative charts, so resampling them needs its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cquil11
force-pushed
the
fix/kv-cache-per-engine-identity
branch
from
August 9, 2026 21:33
19e2099 to
c599977
Compare
Follow-up to the per-engine identity change in this PR, from three
independent audits of the diff against the full production corpus.
Fixes a regression the first commit introduced, proven on replay 654
(dynamo-vllm gb200, 4 decode workers, engine=0..3):
v12 -> [0, 0, 1, 1, 2, 2, 3, 3] (duplicated per phase)
was -> [decode 3, decode 1, decode 0, decode 2]
now -> [0, 1, 2, 3]
Two causes, both fixed:
- The role prefix was applied even when every engine shared one role, so
an aggregated deployment read "decode 0..3" for no reason. The role is
now shown only when engines actually differ in role.
- Sorting keyed off the COMPOSED label, so role-qualified names fell back
to blob order and scrambled the ranks (and with them the palette, which
is indexed by array position). Ordering is now a tuple over the identity
components: role, then numeric rank, then worker, then blob order.
Hardening, each with a test:
- Same-label endpoints are only fused when their values agree. Plain
sglang emits an identity with no distinguishing field at all
({engine_type: unified, model_name, tp/pp/moe_ep_rank all 0}), so two
replicas behind a router would previously have collapsed into one with
the other's samples discarded silently. All 169 sglang rows are
single-endpoint today, so this is a latent hazard, not a live bug.
- The surviving mirror is picked by wall-clock coverage, then sample
count. On replay 815 the old count-only rule was decided by one sample
out of 3865, and a dense-but-truncated mirror could have shortened the
engine's whole series.
- Carry-forward is capped at 5x an engine's own median scrape gap, so a
reporting hole drops the engine out of the mean instead of pinning it
to a stale reading. Real runs sit at 1 Hz with gaps never above ~1 s.
- engineRoleLabel falls through to dynamo_component instead of stopping at
a present-but-unmapped engine_type. Aggregated dynamo-sglang workers
carry engine_type="unified" alongside dynamo_component="backend".
- Blank label values are treated as absent, and the collision qualifier
falls back to a counter rather than appending apostrophes.
Frontend: the average line's name, color, stroke and scatter are driven by
whether the point has multiple engines, so keying that off the phase-sliced
array made the chart change identity between the Warmup and Profiling tabs.
It now keys off the unsliced count, and engine colors come from the unsliced
position so a rank keeps its color across phases.
Ops: the trace-server-metrics route gets maxDuration=300. Every stored row
is stale until the backfill drains it, and the slow path re-parses blobs up
to 448 MB compressed; the platform default would cut those off mid-parse,
and the blob cache only populates on success, so every visitor would re-pay.
Corrections to comments and docs that the audits disproved:
- Warmup and profiling blocks were documented as covering disjoint time
ranges. Their first/last bounds do overlap (replay 820 by 67 s), but
measurement shows only ONE profiling sample lands inside the warmup
window and there are zero exact-instant collisions, so the union is
still sound. Both the code comment and the doc now say that precisely.
- "TP ranks track each other to four decimal places" overstated it: they
agree exactly at 99.7-99.8% of instants and their whole-run means agree
to within 0.25%, with rare single-scrape transients.
- The kvCacheUsageByEngine type doc still described the v12 contract.
Measured after these changes (roughness = mean |tick delta| / stddev):
replay 654 dynamo-vllm/gb200/kimik3 8 -> 4 engines 0.734 -> 0.009
replay 839 dynamo-sglang/h200/glm5.2 64 -> 18 engines 1.003 -> 0.023
replay 813 vllm/mi355x/dsv4 DEP8 8 engines 0.048 (unchanged)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion Two independent replicas behind a round-robin router would have similar means by design and would still be fused into one engine. That degrades the per-engine overlay but not the cluster average, whereas the uneven-load case the threshold does catch is the one that would make the average wrong. Say so at the constant rather than implying the check is airtight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
The KV cache utilization chart on the agentic detail page drew one line per raw metric series rather than one per engine. On DEP points that produced a tangle of duplicate lines over an unreadable legend — the 8-rank DP run at
/inference/agentic/439261drew 32 lines labelledDP 0 DP 0 DP 0 DP 0 DP 1 DP 1 ….229 of the 436 stored rows that carry a per-engine breakdown were affected, at up to 8× duplication (64 lines).
Root cause
A
RawSeriesin theserver_metrics_export.jsonblob is one(scrape endpoint × phase block × label set)tuple — not one engine. Three independent duplications were being folded straight intokvCacheUsageByEngine:CHART_SERIES_VERSIONv12 merged thewarmup_metricsblock intometrics. The aggregate path keys bystart_nsso it stayed continuous, but the per-engine path emitted one entry per series — so every engine appeared at least twice, and single-engine points started drawing a spurious two-line "per-engine" overlay that shouldn't exist at all./metricsendpoint, scraped ~176 ms apart. On 439261, ports:8895and:8896both reportengine="0".."7"with values identical to 4+ decimal places → 8 DP ranks became 16 series.tp_rank=0..7, allmean≈0.05105) looked like 8 distinct engines.The same fragmentation corrupted the cluster-average line.
aggregateByStartgroups on an exactstart_ns, and scrapes from different endpoints/workers never share a nanosecond — so each output tick averaged only whichever subset happened to collide. On a disaggregated run that alternates between "prefill only" (≈0.005) and "decode only" (≈0.104): a full-scale sawtooth, not an average.Secondary: the old
engineLabelfell back toString(idx)— the index in the concatenated series array — so the same engine got a different label in each phase (4in profiling,9in warmup).Fix
Group series by their Prometheus label set (
seriesIdentityKey): the label set is the series identity andendpoint_urlis transport. Deployments whose endpoints really are distinct engines say so in the labels — Dynamo tags every series withworker_id,dynamo_component,engine_type— so prefill rank 0 and decode rank 0 stay separate.tp_rank/pp_rank/ep_rank/moe_ep_rankare excluded from the identity;engine/engine_idx/dp_rankare not, because those do name distinct pools.Within an identity the three duplications get three treatments: phase blocks union by scrape instant, shard ranks at the same instant collapse to their mean, and mirrored endpoints (which overlap in time) resolve to the endpoint with the most complete coverage — merging them instead would interleave near-duplicate samples and halve the effective span of the frontend's fixed-width rolling average.
The cluster average becomes a true mean across logical engines on the union of their scrape instants, each engine holding its last sample until its next one and contributing only inside its own observed window (so an engine that starts late or stops early neither drags the mean toward a stale value nor drops it to zero).
Engines are labelled
0..Nfor bare DP ranks, role-qualified (prefill 0,decode) when the orchestrator gives a role, and worker-qualified (decode 0 (ee1b)) only when two would otherwise collide. The frontend renders bare numerics asDP Nand passes self-describing labels through unchanged.CHART_SERIES_VERSION12 → 13. Runbun run --cwd packages/db db:backfill-chart-seriesafter merge — until then the API serves these points via the slow recompute path (~70 s for the largest blobs).Verification against real data
Recomputed from the actual stored blobs and compared to what v12 wrote. Roughness = mean |tick-to-tick delta| ÷ series stddev; a clean 1 Hz single-grid row sits near 0.18, so a value ≥ 0.6 is sawtooth rather than signal.
Single-endpoint rows come out byte-identical on the average line. Disaggregated rows shift level on purpose: the mean now weights every engine once instead of over-weighting whichever subset shared a timestamp (on 439312, one decode engine at ~0.104 was previously carrying half the average against four prefill engines at ~0.006).
Label quality on 439312:
[0, 0, 1, 1, 2, 2, 3, 3, 4, 9]→[prefill 0, prefill 1, prefill 2, prefill 3, decode].Browser-verified against a local dev server on the real read-only DB (so the API took the live recompute path, not a fixture): 439261 renders 8 distinct DP lines + Avg with a readable 9-entry legend, in both the Profiling and Warmup stages; 436433 renders 4. The disagg per-source selector on 439312 still resolves each worker to a single engine (empty overlay, 4369-point average), confirming the recursive
metricSourcesbranch.Scope deliberately not taken
Sum-combined metrics (queue depth, prefill/decode TPS, prefix-hit rate) sit on the same exact-
start_nsgrouping and show the same fragmentation on disaggregated rows (queue depth on 439312: mean tick delta 2.03 vs stddev 1.41). They are not touched here: their point count feeds running-sum cumulative charts (cumulativeUniqueInputTokensdoessum += value, not a time integral), so resampling them onto a denser timeline would silently change every cumulative total. That needs its own change with its own verification.Review pass
Three independent audits ran over this diff against the full production corpus (449 replay rows, 25 config families). They found one real regression introduced by the first commit and several latent hazards; all are fixed in the second commit.
Regression, proven on replay 654 (dynamo-vllm gb200, 4 decode workers):
[0, 0, 1, 1, 2, 2, 3, 3][decode 3, decode 1, decode 0, decode 2][0, 1, 2, 3]Two causes: the role prefix was applied even when every engine shared one role, and sorting keyed off the composed label so role-qualified names fell back to blob order — scrambling the ranks and, with them, the palette. Ordering is now a tuple over the identity components (role, numeric rank, worker, blob order), and the role is only shown when engines actually differ in role.
Hardening (each with a test): same-label endpoints are only fused when their values agree, so a router-fronted multi-replica topology cannot silently lose an engine — plain
sglangemits an identity with no distinguishing field at all, and all 169 of its rows are single-endpoint today, so this was latent rather than live. The surviving mirror is picked by wall-clock coverage rather than raw sample count (on replay 815 the old rule was decided by one sample out of 3865). Carry-forward is capped at 5x an engine's own median scrape gap.engineRoleLabelfalls through todynamo_componentinstead of stopping at a present-but-unmappedengine_type.Frontend: the average line's name, color, stroke and scatter are all driven by whether the point has multiple engines, so keying that off the phase-sliced array made the chart change identity between the Warmup and Profiling tabs. It now keys off the unsliced count, and engine colors come from the unsliced position.
Ops:
maxDuration = 300on the route. Every stored row is stale until the backfill drains it, the slow path re-parses blobs up to 448 MB compressed, and the blob cache only populates on success — so without this every visitor to a large point would re-pay a request the platform default had already cut off.Corrections to claims the audits disproved. The first commit documented warmup and profiling blocks as covering disjoint time ranges. Their bounds do overlap (replay 820 by 67 s) — but measurement shows exactly one profiling sample lands inside the warmup window and there are zero exact-instant collisions, so the union is sound and both the comment and the doc now say that precisely. "TP ranks track each other to four decimal places" was also overstated: they agree exactly at 99.7-99.8% of instants, with whole-run means within 0.25%.
Post-review measurements:
Known, deliberately out of scope
extractServerMetricSamples(packages/db/src/queries/agentic-aggregates.ts:142) is a second, independent implementation of the same KV averaging and still does the naive exact-start_nsgrouping. It feedsaggregate_stats.kvCacheUtilon the Aggregates tab, so on a disaggregated point that tab can report a median of 0.0% while the Per-point chart sits at ~12%. Both were wrong before; now one is right. Fixing it means aSTATS_VERSIONbump and a second full backfill over the same 7.57 GB, and the right fix is probably to derivekvCacheUtilfromchart_seriesrather than duplicate the logic a third time — so it belongs in its own PR. The two numbers are never on screen at the same time.Separately, the sum-combined metrics (queue depth, prefill/decode TPS) share this bug's root cause and read 0.500x the true value on disaggregated points (measured against an all-worker carry-forward sum; a non-disagg control reads 0.991x). 14 rows affected. Not fixed here because their point count feeds running-sum cumulative charts —
cumulativeUniqueInputTokensdoessum += value, not a time integral — so correcting the instantaneous lines without also converting those helpers to integrate over dt would inflate every cumulative total.Tests
compute-chart-series.test.tscovering mirrored frontends, phase joining, single-engine suppression, disagg worker separation, TP shard collapse, DP-vs-shard disambiguation, collision qualification, and sawtooth-free averaging on unaligned grids.decode) is not prefixed intoDP decode.lint/fmt/typecheck.Overlay (
?unofficialrun=) support is not applicable — this path is the agentic per-point detail view, which loads a single stored trace replay and has no overlay rendering branch.🤖 Generated with Claude Code
Note
Medium Risk
Chart-series algorithm changes affect many stored agentic points until
db:backfill-chart-seriesruns; wrong mirror/replica fusion could mislabel engines, though tests and value-agreement guards mitigate this.Overview
KV cache chart series (v13) no longer treat each raw Prometheus series as its own engine.
compute-chart-series.tsgroups by label identity (seriesIdentityKey), collapsing warmup/profiling duplicates, mirrored vLLM API-server scrapes, and TP/PP/EP shard ranks; clusterkvCacheUsageis averaged across engines on a union timeline with carry-forward and stale-sample limits instead of exactstart_nsgrouping that sawtoothed on disaggregated runs.Display and UI: engines sort by role/rank/worker with smarter labels; the agentic KV card uses unsliced engine count for overlay vs average styling, maps numeric ranks to
DP N, and leaves self-describing labels likedecodeunprefixed.trace-server-metricssetsmaxDuration = 300for slow recompute on pre-backfill rows. Docs add a “Logical Engines vs Raw Series” section; extensive unit and Cypress coverage exercises the new identity rules.Reviewed by Cursor Bugbot for commit 1aedd4d. Bugbot is set up for automated code reviews on this repo. Configure here.