feat(subscriber): read engine load via GetLoads gRPC (CAC-235)#160
feat(subscriber): read engine load via GetLoads gRPC (CAC-235)#160EdHasNoLife wants to merge 9 commits into
Conversation
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
…ucible vendored proto - Clamp external token_usage/cache_hit_rate to finite [0,1] so garbage from the engine can't overflow cache_memory_bytes or poison hit_rate (NaN-safe). - Extract buildStatsScraper so the gRPC-vs-HTTP load-source selection is unit tested; add the control-flow test. - Rename GRPCLoadsScraper* to the Go initialism; log the selected load source. - Add proto-gen-vendored (run by proto-gen) + go:generate + CI drift check so the vendored engine stubs are reproducible and can't silently drift. - Document the selectable gRPC stats path and the deferred sidecar auto-inject of --engine-loads-grpc (gated on the engine GetLoads floor) in the wiring doc.
Codex reviewFiles reviewed
FindingsBlocking
Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
Mapping a non-finite cache_hit_rate to 0 and still counting it dragged the mean down and contradicted the 'ranks that report one' comment. Exclude non-finite ranks from the average instead; finite out-of-range values are still clamped and kept. token_usage keeps finite01 (0 never wins its max).
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
…o provenance The generated vllmengine/*.pb.go were counted as hand-written by cover-check; add the package to COVER_EXCLUDE (it is entirely generated). Also strengthen the proto provenance note: the fields are copied verbatim from the SMG source and a re-sync must pin the upstream revision — the bufconn test proves self-consistency with the generated stubs, not a match to the deployed engine.
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
… stale doc note - Record the exact machxai/smg commit (6d63ec8, 2026-07-09) the vendored proto was copied from, verified field-for-field against upstream. - Add pkg/adapters/engine/vllmengine to the pre-pr generated-drift paths so the local gate inspects the stubs it now regenerates (matches the CI check). - Reword the wiring-doc note: the enumerated stats flags are recognised by the binary; they are simply not set by the sidecar yet.
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fixNone. Nit
Per-category coverage
Verdictapprove-with-nits |
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
float64(CacheSizeBytes) can round above the int64 range at extreme capacities, so int64(usage*float64(cap)) could overflow to a negative. Clamp in float space before the conversion and return the exact int64 capacity at the top end. Adds a MaxInt64 boundary test.
Codex reviewFiles reviewed
|
|
Re: the Codex finding that Tracked as CAC-249 — deferred deliberately, not wired here:
Ships once the engine floor moves. The subscriber capability lands in this PR; the operator wiring follows in the ticket. |
… stats-path docs source-neutral The overflow test's 'got > math.MaxInt64' is always false for an int64 (SA4003, failing ci-lint) — the meaningful guard is got < 0 (overflow wraps negative), and the next assertion already pins the exact MaxInt64 value. Also update doc.go + StatsReporter doc to describe the selectable load source (HTTP /metrics or GetLoads gRPC), not HTTP-only (review nit).
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
…ntract (drop SMG references) inference-cache is a general repo, so its comments shouldn't name our downstream gateway fork. Reframe the GetLoads load source + vendored proto as the vLLM engine gRPC contract (vllm.grpc.engine.VllmEngine / vllm.entrypoints.grpc_server); drop the machxai/smg provenance + the SMG-servicer version floor (keep the GetLoads + vLLM >= 0.19 requirement). Regenerated the vendored stubs from the updated header.
Codex reviewFiles reviewed
FindingsBlockingNone. Should-fix
Nit
Per-category coverage
Verdictchanges-requested |
Gives the kvevent-subscriber a gRPC-native load source: read the engine's
GetLoadsRPC and projectSchedulerLoadintoReplicaStats, as an alternative to scraping HTTP/metrics.Why: a vLLM engine served over gRPC (
vllm.entrypoints.grpc_server) exposes no HTTP/metrics— live load is available only overGetLoads. Today the subscriber only scrapes HTTP, so IC gets no load and the ranker routes on residency alone. This is the clean long-term fix: structured data, no text round-trip, no extra container.Design:
GRPCLoadsScraperimplements the existingstatsScraperinterface, soStatsReporteris unchanged. Selected by a new--engine-loads-grpc <host:port>flag (empty = HTTP scrape, unchanged default). Mapping mirrors the HTTP scraper: request counts topressure = clamp01((running+waiting)/ceiling),token_usagetocache_memory_bytes(via--engine-cache-size-bytes),cache_hit_ratetohit_rate; across DP ranks counts SUM, usage MAX, hit-rate MEAN (finite values only).Vendored proto: a minimal
GetLoads-only subset of the vLLM engine gRPC contract (pkg/adapters/engine/vllmengine/), package keptvllm.grpc.enginefor wire compat, deliberately outsideproto/so it isn't subject to IC's buf-lint / drift checks (it's a vendored external contract, not IC's API). Regenerable viamake proto-gen-vendored; CI drift-checks it.Caveat:
GetLoadscarries no external-tier (T2/LMCache) token counters, so thoseReplicaStatsfields stay HTTP-/metrics-only in this mode; the load signals the ranker uses (pressure, cache-usage, hit-rate) are all provided.Tests: mapping (single/multi-rank, zero-config, sanitization, overflow, error) + a bufconn end-to-end through the generated stubs + load-source control-flow selection. Full
pkg/adapters/enginesuite passes. Live end-to-end validation is pending a GetLoads-capable (vLLM >= 0.19) engine.CAC-235.