feat: ingest and display kv_transfer_lib on benchmark results#516
Open
Oseltamivir wants to merge 2 commits into
Open
feat: ingest and display kv_transfer_lib on benchmark results#516Oseltamivir wants to merge 2 commits into
Oseltamivir wants to merge 2 commits into
Conversation
The benchmarking repo now derives the KV-cache transfer library (Mooncake, NIXL, MoRI-IO, UCX) at result-processing time and emits an optional `kv_transfer_lib` string on disaggregated result rows. Until now that detail was invisible downstream — e.g. dynamo-sglang covers both Mooncake (DSV4) and NIXL (GLM-5 STP) recipes under one framework key. Forward-only ingestion: - Migration 008: nullable `kv_transfer_lib` column on benchmark_results (result-level metadata, not part of the config natural key, so config identity and trend-line continuity are unchanged), and recreates the latest_benchmarks materialized view to expose it. - ETL: benchmark-mapper surfaces the field as a metrics sibling (lowercased, defensively narrowed); benchmark-ingest writes it. - Queries/API: all benchmark SELECTs and the json-provider return it. - UI: tooltips show a "KV Transfer" line in the official, overlay (unofficial run), and GPU-comparison views. NULL/absent renders nothing — unknown is never guessed. Historical rows stay NULL (the runner didn't emit the field); the tooltip simply omits the line for them. AMD history is derivable from the framework column if a backfill is ever wanted.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…eries
Migrations are applied manually (pnpm admin:db:migrate), separately from
the Vercel deploy, so the PR preview runs against a DB that doesn't have
migration 008 yet. A bare reference to a non-existent column fails to
plan ("column \"kv_transfer_lib\" does not exist"), 500ing every
benchmarks request and blanking the dashboard — the same failure mode as
the migration-006 workers rollout.
Read the column via to_jsonb(row) ->> 'kv_transfer_lib' in all four read
paths (getLatestBenchmarks base-table + view branches,
getBenchmarksForRun, getAllBenchmarksForHistory). JSON key lookup is a
runtime op that returns NULL for an absent key instead of failing at
parse time; behavior is identical once the column exists.
Adds a regression test asserting the read queries never reference the
column directly. The ingest INSERT still requires migration 008 — run it
before the first ingest of field-bearing artifacts.
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 benchmarking repo is gaining runtime detection of the KV-cache transfer library (Mooncake, NIXL, MoRI-IO, UCX) for disaggregated runs, emitted as an optional
kv_transfer_libstring on result rows (companion InferenceX PR, branchfeat/kv-transfer-lib-metadata). This PR is the forward-only app side: ingest the field when present and show it in chart tooltips. Until now the transfer library was invisible downstream —dynamo-sglangcovers both Mooncake (DSV4 recipes) and NIXL (GLM-5 STP recipes) under one framework key; onlymooncake-atom/mori-sglangencoded it in the name.Changes
DB (
packages/db)kv_transfer_lib textcolumn onbenchmark_results(+ lowercase CHECK). Result-level metadata, deliberately not part of the config natural key — config identity and trend-line continuity are unchanged. Recreates thelatest_benchmarksmaterialized view (it materializesselect br.*at creation time) with the identical migration-007 definition.benchmark-mapper: surfaces the field as a metrics sibling (likeworkers), trimmed/lowercased, defensively narrowed — absent/empty/non-string ⇒ undefined.benchmark-ingest: writes the column (NULL when absent), included in the conflict-update path.App (
packages/app)BenchmarkRow→AggDataEntry→InferenceDatapassthrough.unofficial-runroute carries the field from raw artifacts so overlays match the official path.Unofficial run overlay support
Works for both official runs and
?unofficialrun=overlays: the overlay path reusesmapBenchmarkRow+transformBenchmarkRows, and the route copieskvTransferLibinto the overlayBenchmarkRows. Covered by unit tests on the overlay route (route.test.ts) and the overlay tooltip generator (tooltip-utils.test.ts). Note: live manual verification isn't possible yet — no artifacts contain the field until the benchmarking-repo PR merges and a run lands; the overlay tests pin the behavior in the meantime.Existing data
Historical rows stay NULL and the tooltip simply omits the line. If backfill is ever wanted, AMD history is derivable from the framework column alone (
mooncake-atom⇒ mooncake,mori-sglang⇒ mori,vllm-disagg⇒ nixl) in one UPDATE; recipe-drivendynamo-*history would need a git-history walk and is intentionally out of scope.Testing
pnpm typecheck,pnpm lint,pnpm fmtclean.use-feature-gate/chunk-load-recovery/visit-trackerfailures reproduce on clean master and are unrelated.Sequencing
to_jsonb(row) ->> 'kv_transfer_lib'(the migration-006workersrollout pattern), so previews and prod work whether or not migration 008 has run. A regression test pins this.Note
Medium Risk
Touches benchmark ingest and all benchmark read queries plus a materialized view rebuild; mis-sequenced migration could 500 the dashboard, though the to_jsonb guard and optional field limit blast radius.
Overview
Adds optional KV-cache transfer library metadata (
mooncake,nixl,mori,ucx) for disaggregated benchmark runs, from runner artifacts through the DB to chart tooltips—without changing config identity or trend lines.Database: Migration 008 adds nullable
kv_transfer_libonbenchmark_results(lowercase CHECK), rebuildslatest_benchmarks, and the ingest path maps, normalizes, and upserts the field separately from scalarmetrics(same pattern asworkers).API reads: Benchmark SELECTs expose the value via
to_jsonb(row) ->> 'kv_transfer_lib'so queries still plan when migration 008 is not applied yet.App: The field flows
BenchmarkRow→ chart points → KV Transfer in official, unofficial overlay, and GPU date-comparison tooltips (known labels mapped; absent/null omits the line). The unofficial-run overlay route passes it through from artifacts.Reviewed by Cursor Bugbot for commit 22a039c. Bugbot is set up for automated code reviews on this repo. Configure here.