Skip to content

PR: wire prefix-cache hit counters into /metrics - #814

Open
1571859588 wants to merge 2 commits into
pegainfer-project:mainfrom
1571859588:feat/sim-frontend-prefix-cache-metrics
Open

PR: wire prefix-cache hit counters into /metrics #814
1571859588 wants to merge 2 commits into
pegainfer-project:mainfrom
1571859588:feat/sim-frontend-prefix-cache-metrics

Conversation

@1571859588

Copy link
Copy Markdown

Summary

Expose real prefix-cache query/hit counters end-to-end so they surface in the
vLLM-compatible /metrics endpoint, matching vLLM's prefix_cache_stats
semantics (token granularity).

  • openinfer-engine: add prefix_cache_queries / prefix_cache_hits to LoadSnapshot.
  • openinfer-vllm-frontend (bridge.rs): map the new LoadSnapshot fields into
    SchedulerStats.prefix_cache_stats.
  • openinfer-qwen3 scheduler: aggregate cached_tokens from prefill resolution
    into per-step StepEffects and accumulate across both scheduler loops
    (non-LoRA and LoRA), publishing real query/hit counts.
  • openinfer-qwen35: carry the counters through its scheduler publish path.
  • openinfer-sim (frontend_e2e): assert the counters are emitted.

Verified

Built and tested on A100 (SM 80, CUDA 12.4, nightly rust), card 5:

  • cargo build --release (qwen3) — OK
  • cargo test --release --workspace --lib — 74 passed
  • cargo test --release -p openinfer-vllm-frontend — 27 passed
  • cargo test --release -p openinfer-sim --test frontend_e2e — 13 passed (real GPU)
  • cargo fmt --all -- --check — clean

Notes

Cargo.lock reflects the verified local dependency resolution (bindgen 0.72.1)
required to build qwen3 on this environment.

Test plan

  • CI full CUDA build (qwen3) green
  • prefix_cache_queries / prefix_cache_hits present in /metrics under load
  • multi-turn repeat-prompt request shows hits > 0

The upstream engine-core-client already exposes vllm:prefix_cache_queries
and vllm:prefix_cache_hits counters (from SchedulerStats.prefix_cache_stats),
but openinfer's publish_scheduler_stats bridge left prefix_cache_stats
zeroed via ..SchedulerStats::default(), so the counters stayed at 0.

Wire the data through:
- LoadSnapshot gains prefix_cache_queries / prefix_cache_hits fields
  (openinfer-engine, our own type).
- bridge.rs maps them into SchedulerStats.prefix_cache_stats.base so the
  upstream gauges reflect real hits instead of being silent.
- All LoadSnapshot construction sites stay compatible via ..Default::default()
  (schedulers do not yet feed real values; counters read 0 until they do,
  which requires A100-node verification of the scheduler crates).

Tests:
- bridge/tests.rs asserts the mapped prefix_cache_stats.base values.
- sim frontend_e2e metrics test publishes a snapshot with prefix fields and
  asserts vllm:prefix_cache_queries / vllm:prefix_cache_hits surface at 100/80.

Verified: cargo fmt clean; cargo clippy --release --locked -p
openinfer-engine -p openinfer-vllm-frontend -p openinfer-sim clean;
cargo test --release (engine 27 / frontend 13 / sim 3) pass.

Part of C1 (prefix-cache observability). Independent of PR pegainfer-project#808.
Wire cached_tokens from prefill resolution into per-step effects and
accumulate across both scheduler loops so the published LoadSnapshot
reflects actual prefix-cache queries/hits (token granularity), matching
the vLLM prefix_cache_stats semantics introduced in 16f52fa.

Cargo.lock reflects the verified local dependency resolution (bindgen
0.72.1) used to build qwen3 on A100.
Copilot AI review requested due to automatic review settings August 1, 2026 05:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR wires prefix-cache query/hit counters from model schedulers into LoadSnapshot, maps them through the vLLM frontend bridge into SchedulerStats.prefix_cache_stats, and adds tests to ensure the /metrics endpoint exposes the expected vLLM-compatible prefix-cache gauges.

Changes:

  • Extend openinfer-engine::LoadSnapshot with prefix_cache_queries / prefix_cache_hits.
  • Map the new LoadSnapshot fields into SchedulerStats.prefix_cache_stats in the vLLM frontend bridge.
  • Add scheduler-side aggregation (qwen3) and end-to-end metric assertions (sim + bridge tests).

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
openinfer-engine/src/engine.rs Adds prefix-cache query/hit counters to the scheduler load snapshot contract.
openinfer-vllm-frontend/src/bridge.rs Maps LoadSnapshot prefix-cache counters into vLLM SchedulerStats.prefix_cache_stats.
openinfer-vllm-frontend/src/bridge/tests.rs Extends bridge tests to assert prefix-cache counters are forwarded into scheduler stats.
openinfer-sim/tests/frontend_e2e.rs Adds end-to-end assertions that /metrics surfaces prefix-cache counters.
openinfer-qwen3/src/scheduler/effects.rs Extends per-step effects to carry prefix-cache query/hit deltas.
openinfer-qwen3/src/scheduler/resolve.rs Aggregates cached-token info into per-step prefix-cache effects during prefill resolution.
openinfer-qwen3/src/scheduler.rs Accumulates per-step prefix-cache effects into lifetime counters and publishes them via LoadSnapshot.
openinfer-qwen3/src/scheduler/tests.rs Updates scheduler tests for new StepEffects fields.
openinfer-qwen35/src/scheduler.rs Updates LoadSnapshot publication to remain compatible with new fields via Default.
openinfer-glm52/src/scheduler/load.rs Updates LoadSnapshot publication to remain compatible with new fields via Default.
Cargo.lock Updates dependency resolution (notably bindgen) to match the verified build environment.
Suppressed comments (3)

openinfer-qwen3/src/scheduler.rs:741

  • These counters are accumulated with += on a u64, which will wrap on overflow in release builds. Since this value is surfaced as a long-lived metric, prefer saturating arithmetic to avoid silent wraparound.
                prefix_cache_queries += effects.prefix_queries;
                prefix_cache_hits += effects.prefix_hits;

openinfer-qwen3/src/scheduler.rs:773

  • These counters are accumulated with += on a u64, which will wrap on overflow in release builds. Since this value is surfaced as a long-lived metric, prefer saturating arithmetic to avoid silent wraparound.
        prefix_cache_queries += effects.prefix_queries;
        prefix_cache_hits += effects.prefix_hits;

openinfer-qwen3/src/scheduler.rs:952

  • These counters are accumulated with += on a u64, which will wrap on overflow in release builds. Since this value is surfaced as a long-lived metric, prefer saturating arithmetic to avoid silent wraparound.
        prefix_cache_queries += effects.prefix_queries;
        prefix_cache_hits += effects.prefix_hits;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +133 to +136
// First chunk is the only place a request counts toward the
// prefix-cache query total; its cached token span is the hit total.
effects.prefix_queries += 1;
effects.prefix_hits += result.cached_tokens as u64;
Comment on lines +96 to +98
/// Prefix-cache queries counted this step: one per request whose first
/// prefill chunk ran (a `queries` increment in vLLM terms). Carried into
/// `LoadSnapshot.prefix_cache_queries` for the vLLM frontend metrics.
Comment on lines +604 to +605
prefix_cache_queries += effects.prefix_queries;
prefix_cache_hits += effects.prefix_hits;
Comment on lines 842 to 845
num_running_reqs: (active.len() + prefilling.len()) as u64,
num_waiting_reqs: num_waiting_reqs as u64,
..Default::default()
});

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f24658e45

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +692 to +693
queries: snapshot.prefix_cache_queries,
hits: snapshot.prefix_cache_hits,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Send prefix-cache deltas to the vLLM counters

The Qwen3 scheduler publishes prefix_cache_queries/hits as lifetime totals on every load-watch update, but these values are placed directly into SchedulerStats, whose Prometheus path increments counters from each stats message. For a request that then decodes many tokens, every scheduler snapshot re-adds the same accumulated prefix-cache totals, so /metrics grows with the number of decode/idle publishes rather than the number of cache lookups. Publish per-update deltas here (or have the bridge subtract the last seen snapshot) before feeding the vLLM counters.

Useful? React with 👍 / 👎.

});
// First chunk is the only place a request counts toward the
// prefix-cache query total; its cached token span is the hit total.
effects.prefix_queries += 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Count prefix-cache queries by tokens

This increments the query counter once per request while prefix_cache_hits is already a token count from cached_tokens; vLLM's prefix-cache query metric is token-granularity. With any prompt longer than one token, especially a warm 1k-token prompt with hundreds of cached tokens, the exported counters can report far more hits than queries and make hit-rate calculations nonsensical. Add prompt_len for the first chunk instead of 1 so the denominator matches the hit units.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

This pull request has been inactive for 14 days. It will be closed after another 30 days unless there is new activity.

@github-actions github-actions Bot added the stale Automatically marked after inactivity label Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Automatically marked after inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants