Skip to content

feat(radix-index): shared prefix-cache index service - #2437

Open
slin1237 wants to merge 3 commits into
fix/radix-tree-age-evictionfrom
feat/radix-index
Open

slin1237 wants to merge 3 commits into
fix/radix-tree-age-evictionfrom
feat/radix-index

Conversation

@slin1237

@slin1237 slin1237 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Note

2 of 3 in the shared prefix-cache index stack. Base: fix/radix-tree-age-eviction (#2500); #2436 (the data structure) is merged. This PR's first commit adds the tree's coverage/runs reads the service needs. Next: the gateway integration (#2438). No gateway code changes here.

Description

Problem

Each gateway builds its own picture of which worker holds which prefix, from the fleet's KV-event streams. That fragments as gateways scale out: a conversation's next turn often lands on a gateway that never saw turn 1, and it re-prefills. Measured on an agentic workload: at 8 gateways, per-gateway cache hit drops to 0.66 (routing precision 0.12).

Solution

smg-radix-index: one shared index the gateways query and feed over gRPC, so every gateway sees the same fleet-wide view. Design points, deliberately few:

flowchart LR
  G["gateway(s) — the only clients"] -- "① overlap query, 2ms deadline" --> R0[("replica 0")]
  G -- "② placement after each request<br/>③ worker add / drop" --> R0
  R0 <-- "relay: state changes only" --> R1[("replica 1")]
  R1 -. "bootstrap Pull" .-> R0
Loading
  • One keyspace per model × symbol_kind × block_size, one RadixTree each, per-keyspace locks, idle-keyspace GC.
  • Two feeds, opposite shapes. Placements (the prompt's block chain → the worker that served it) are unsequenced and idempotent: any gateway may publish, duplicates collapse on a shared-lock fast path, and a re-publish of an established chain can go as a {tip, len} digest. Events (worker KV events) are sequenced ground truth with epochs and a single owner per worker — optional, via the bridge; ~50–500× the volume.
  • Replicas copy, they never agree. An apply is relayed only when it changed state, so an echo dies in one hop; a new replica bootstraps by Pull, streamed one holder at a time so the serving replica never holds a second copy of its index; lifecycle add/drop (applied to every keyspace of the model the worker appears in) retire dead workers; the silence backstop soft-retires (descore, keep state) so a late-healing event batch restores the holder, and only a second silence window retires it; idle placement-fed holders retire so their keyspaces can be collected.
  • Queries fall back, never block: hard deadline, advisory answers.
  • Capacity guard with hysteresis. A placement-fed holder past 2× its declared capacity is cut back to 1× (depth-ordered, prefix-closed), not to the bound: cutting to the bound on every apply made each fresh chain added-then-cut, so its relay re-applied as changed on the peer and echoed back forever — a permanent apply storm between replicas at 100% CPU with no external traffic, found by the end-to-end failover drill and reproduced with two bare replicas. The cut itself is recency-ordered (evict_oldest, feat(radix-tree): recency-ordered whole-chain eviction #2500): the holder's least-recently-published chains go first, whole, and a chain whose descendants are still being published is as young as they are; the shared-lock duplicate walk counts as a publish. Depth-ordered truncation was the first version and, under a placement feed held above capacity, dropped the freshest long-prompt tails and never freed a chain slot — the 30-minute soak measured replica RSS 120 → 460 MiB at a flat block count; with whole-chain eviction the in-process probe holds live chains flat (1.5k, was 1k → 14k) and RSS plateaus.
  • Anti-entropy bounds divergence. Relay is best-effort, so a partition or a wedged peer left replicas diverged for good once the missed deltas were gone — the end-to-end partition drill measured every holder differing between two event-fed replicas after a 45 s partition healed (the stale side kept blocks the worker had evicted). Peers now exchange per-holder digests (epoch, seq, block count, order-independent set digest) every --anti-entropy-secs (default 15) and pull, wholesale, each holder where the sibling is provably ahead: a higher watermark, or a different block set at the same watermark for sequenced (event-fed) holders. Placement-fed holders are pulled only when missing — replicas copy them, never agree on them, by design. New RPCs Digests / PullHolders; counters radix_index_anti_entropy_{rounds,holders_pulled}_total; a live test where one replica's only link to the other is anti-entropy, including a removal it never saw as an event.
  • Lanes and intervals keep the index engine-neutral. A worker's cache is not always one contiguous prefix: hybrid models keep several independently evicted position sets per worker (full attention, a sliding window that frees its old blocks, recurrent state saved at checkpoints), and the reusable prefix is the largest position every set accepts at that position. The index stores each such set as its own holder (worker#lane, with the publisher's opaque description on Added.metadata, kept, snapshotted and echoed, never parsed) and answers every covered run of the query per holder (HolderScore.intervals) next to the contiguous depth, from one coverage walk. The reuse rules live in the gateway (feat(router): query and feed the shared prefix-cache index from the policy layer #2438). Splitting an engine's event stream into lanes is the publisher's job (the vLLM group-events work in feat(kv-events): route using reported KV cache groups #2497), not the index's; lifecycle control for a worker fans out to its lanes. Two latent bugs found by the lane tests and fixed: a snapshot of a holder whose coverage starts past position 0 landed at position 0 on a new lineage on the puller (any bootstrap or anti-entropy pull of an event-fed holder with mid-chain removals); snapshots are now per run and path-prefixed. And replica digests hashed contents only, so that divergence was invisible; they are position-bound now.
  • Misconfiguration fails loudly: unknown or unparsable flags abort startup (all four binaries share one strict parser accepting --flag value, --flag=value and bare switches), an unsupported hash scheme fails the publish stream, and a Stored the tree refuses is never acked as applied.

Changes

  • src/engine.rs — keyspaces, epochs/seq dedup, placement fast paths, digests, TTL/GC, snapshot reconstruction.
  • src/server.rs — gRPC Publish (batched apply + relay) / Subscribe (queries) / Pull (bootstrap); admin plane.
  • src/client.rs — the gateway-side client: deadline query, fire-and-forget placements, bounded lifecycle sends, reconnect.
  • src/bridge.rs — optional worker-event bridge (epoch adoption from acks; digest cache nested (holder, tip) → keyspace so plan and resend are each one lookup).
  • src/wire_hash.rs — versioned wire hash scheme, golden-pinned against kv_index.
  • src/cli.rs — the strict flag parser both binaries use.
  • src/bin/{service,bridge,bench,loadbench,dump}.rs, proto/, deploy/statefulset.yaml, README.md. radix-index-dump pulls one replica's state and prints per-holder block counts and order-independent digests, so two replicas can be diffed for convergence. /metrics also exposes radix_index_apply_duration_seconds and radix_index_query_duration_seconds histograms (engine time per update / per query; a 2 ms bucket edge so "over the gateway's deadline" reads straight off them).
  • tests/ — live two-replica convergence, client fault paths (timeout/disconnect via a never-answering mock), lifecycle drop/re-add and digest miss→resend end to end.
  • Workspace member + dependency entry; release workflow tier 3 (depends on smg-radix-tree and smg-grpc-client).

Suggested reading order: README.mdengine.rsserver.rsclient.rstests/.

Test Plan

  • Unit + live tests as above (42 lib, 10 integration), plus targeted regression locks for the fault-tolerance perimeter: concurrency races (placement split vs. clear, GC vs. placement), epoch/reconnect adoption arithmetic (in the wire-sequence domain: worker batch N rides as seq N+1 so batch 0 is never the unsequenced sentinel), digest-confirm overflow, digest cache keyed per holder and keyspace, lifecycle relay echo suppression and cross-keyspace reach, Cleared on an empty holder, snapshot chunking and per-holder streaming on bootstrap, keyspace GC of placement-only fleets. An audit of that perimeter found and fixed 8 real bugs before this PR (listed in the commit history).
  • Write scaling (loadbench, isolated): placements at the reference rate with 1.4× query-p99 isolation; events at the ~20k-worker rate (~1M blk/s) at 1.16×; apply ceiling 76.8M blk/s.
  • Fault drills via the harness (feat(sim): no-GPU simulation harness for cache-aware routing and the shared index #2439): 45 s partition, wedged replica, kill+relaunch, flap, forced relay overflow, replica-added-under-load — zero request errors.

Validation (end-to-end campaign, 2026-09-08)

  • 55 unit + live integration tests. Live gRPC publish path: 12.5M blocks/s on a single publisher stream against an external service.
  • loadbench at the reference rate (16 publishers × 200 updates/s, external service), query isolation p99 loaded / p99 idle (goal G2 ≤ 2.0): placements 1.53, placements+digest 1.28, events 1.67; query p50 34 µs, loaded p99 70–95 µs; 0.83M blocks/s placements, 0.82M blocks/s events. (At the unthrottled hammer rate — 43M blocks/s — the ratio is 65–129×; that regime is not the gate.)
  • Fault drills through the harness (feat(sim): no-GPU simulation harness for cache-aware routing and the shared index #2439), 8 gateways × 120 mock workers, 305 sessions/s, placement feed, 68,392 requests per leg, 0 errors in every leg:
    • kill replica 0 @60 s, relaunch @90 s bootstrapping from replica 1: follow-up cache at the floor during the blackout (fast-fail, 100% remote_disconnected), 0.78 ten seconds after relaunch, 0.86 at +20 s, 0.94–0.97 by the end; index timeouts 1–2% throughout.
    • replica added under load (3rd replica @60 s): 0.884 follow-up cached, 0.98 same-worker, flat through the join.
    • replica 1 flapped ×3: 0.882 / 0.976, flat.
    • inter-replica partition 45 s and replica-1 SIGSTOP 45 s: 0.882 / 0.976–0.980, no visible dip on the queried replica.
    • staleness (event feed, injected apply lag 30 / 300 / 3000 ms on Stored, 3000 ms on Removed): follow-up cached 0.875 / 0.888 / 0.834 / 0.820; prediction error p95 0 / 0 / 512 / 256 tokens.
  • Found and fixed by the drills: the placement-feed capacity guard produced a permanent relay storm between replicas once a holder reached 2× capacity (see the hysteresis commit note); the drill numbers above are post-fix.

Consistency and memory reruns (2026-09-14, after anti-entropy and the recency-ordered cut)

Same drills as above, 8 gateways × 120 mock workers, seed 42, both replicas dumped at exit and diffed per holder (radix-index-dump):

leg feed before after
inter-replica partition 45 s events 120/120 holders differing (887k vs 498k blocks; the stale side kept blocks the worker had evicted) 0/120, identical totals (555,196 blocks each); anti-entropy 12–13 rounds, 89 + 203 holders pulled
rolling replica restart placements 78 and 27 differing (two seeds) 33 differing, every one in the 1×–2× capacity band
half the gateways cut from replica 0 placements 0 and 33 differing 24 differing, every one in the capacity band

Event-fed holders are sequenced ground truth and now converge exactly. Placement-fed holders are copied, never agreed on: each replica runs its own capacity cut, so two replicas that both hold a worker above its capacity (block counts 4,750–9,371 against a 4,688-block capacity in these legs, e.g. 9,371 vs 4,740 = one replica just cut, the other has not yet) legitimately differ by when they cut. The harness now classifies that separately from a placement holder differing under capacity, which would be a lost update; none occurred.

Placement soak, 30 minutes, after the recency-ordered cut (#2500). Replica RSS on the placement feed: 43 MiB at 1 min, 108 MiB at 10 min, then flat at 102 MiB through minute 30 at 815k–890k resident blocks. Before the cut change the same soak reached 486 MiB at minute 30 and was still climbing (chain slots were never freed). Cut cost measured live with the new radix_index_capacity_cuts_total / radix_index_capacity_cut_seconds_{total,max} counters under loadbench at 450 placements/s across 120 workers held at 2× capacity: 11 cuts/s, worst 3.0 ms, 0.7% of wall time under the write lock, 17 queries over 2 ms out of 19.0M (the depth-ordered cut, same load: 113 of 20.5M).

Caveat on the rerun hit rates. The reruns were made on a laptop shared with other active agent sessions (a host-wide sampler caught a Go build at 460% CPU, rustfmt and clippy-driver at 95–99%, and a Codex process at 30–90% during the runs). Every hit-rate dip in them lines up with such a burst to the minute: index lookups timed out on all eight gateways at once while the service's own query time stayed under 0.1 ms p99, and the timeouts precede the routing errors they cause. The end-state verdicts above (memory, convergence, cut cost) do not depend on that; the follow-up cache ratios quoted in the Validation section are from the quiet-host campaign of 2026-09-08/10 and stand as the reference.

Status

  • No auth/TLS; single-box validation only. Copy-replication caps event-feed write throughput at one instance; holder sharding is the future lever.
  • Digest publishing is opt-in (RADIX_CLIENT_DIGEST=1): bounded by entry count, and every digest misses for event-fed holders. Off by default.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

📝 Summary

Summary by CodeRabbit

  • New Features

    • Added the Radix Index service for publishing, querying, subscribing to, and retrieving indexed content.
    • Added token- and byte-based keyspaces, replication, recovery, and remote client connectivity.
    • Added state inspection, bridge, service, benchmarking, and load-testing tools.
    • Added health, readiness, and Prometheus metrics endpoints.
  • Deployment

    • Added a reference Kubernetes deployment for a two-replica Radix Index setup.
  • Documentation

    • Added usage, architecture, configuration, deployment, metrics, and operational guidance.

Walkthrough

Added the smg-radix-index crate with protobuf contracts, hashing, gRPC service and client components, worker bridging, replication, operational binaries, deployment configuration, documentation, and end-to-end tests.

Changes

Radix index contracts and packaging

Layer / File(s) Summary
Contracts and crate packaging
Cargo.toml, crates/radix_index/Cargo.toml, crates/radix_index/proto/*, crates/radix_index/build.rs, crates/radix_index/README.md, crates/radix_index/deploy/*, .github/workflows/release-crates.yml, scripts/check_release_versions.sh
Adds the workspace crate, protobuf service and messages, generated gRPC bindings, deployment reference, crate documentation, and Tier 3 release registration.
Wire hashing and update conversion
crates/radix_index/src/lib.rs, crates/radix_index/src/wire_hash.rs
Adds hash scheme v1, placement chains, public hash types, and conversions between protobuf updates and engine updates.
Bridge and remote client flows
crates/radix_index/src/bridge.rs, crates/radix_index/src/client.rs
Adds worker event replay, epoch tracking, digest recovery, reconnecting publish and subscribe streams, lifecycle updates, and token or byte queries.
gRPC service and runtime controls
crates/radix_index/src/server.rs, crates/radix_index/src/cli.rs, crates/radix_index/src/bin/service.rs
Adds publish, subscribe, pull, peer relay, bootstrap, readiness, health, metrics, shutdown, and strict CLI handling.
Operational binaries and benchmarks
crates/radix_index/src/bin/bridge.rs, crates/radix_index/src/bin/bench.rs, crates/radix_index/src/bin/loadbench.rs, crates/radix_index/src/bin/dump.rs
Adds the worker bridge executable, local scale benchmark, configurable event or placement load benchmark, and replica dump utility.
End-to-end validation
crates/radix_index/tests/*
Tests client fault outcomes, keyspace isolation, lifecycle recovery, digest replay, worker bridging, placement queries, and replica convergence.

Priority: ➖ Normal

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Worker
  participant Bridge
  participant IndexService
  participant RemoteIndex
  Worker->>Bridge: Stream KV event batches
  Bridge->>IndexService: Publish sequenced updates
  IndexService-->>Bridge: Publish acknowledgements
  RemoteIndex->>IndexService: Subscribe query
  IndexService-->>RemoteIndex: Holder scores
Loading

Merge Risk: 🟠 High · up to 61177

The service can still retain stale placement state, start with incomplete replica state, accept unsafe configuration, and expose mutation APIs without isolation. These risks should be resolved before deployment or merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 74.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 116 functions across 17 files. (1 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the primary change: adding a shared prefix-cache index service for radix-index.
Description check ✅ Passed The description directly explains the shared gRPC index service, its components, behavior, validation, limitations, and relation to the changeset.
Full details: Docstring Coverage

Explanation

Docstring coverage is 74.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 116 functions across 17 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/radix-index

Comment @coderabbitai help to get the list of available commands.

Comment thread crates/radix_index/deploy/statefulset.yaml
Comment thread crates/radix_index/src/bin/bridge.rs Outdated
Comment thread crates/radix_index/src/engine.rs
Comment thread crates/radix_index/src/bridge.rs Outdated
Comment thread crates/radix_index/src/client.rs
Comment thread crates/radix_index/src/bridge.rs Outdated
Comment thread crates/radix_index/src/bridge.rs Outdated
Comment thread crates/radix_index/src/server.rs Outdated
Comment thread crates/radix_index/src/server.rs
Comment thread crates/radix_index/src/engine.rs
Comment thread crates/radix_index/src/engine.rs Outdated
Comment thread crates/radix_index/src/bridge.rs Outdated
Comment thread crates/radix_index/src/engine.rs Outdated
"# TYPE radix_index_capacity_cut_seconds_total counter\n",
"radix_index_capacity_cut_seconds_total {}\n",
"# TYPE radix_index_capacity_cut_seconds_max gauge\n",
"radix_index_capacity_cut_seconds_max {}\n",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: radix_index_capacity_cut_seconds_max is a lifetime high-water mark exported as a gauge, so the tail signal it exists to provide dies after the first pathological cut.

ns_max is only ever fetch_max'd (engine.rs:800) and never reset, so the series is monotonic for the life of the process. Any windowed query an operator would actually write — max_over_time(radix_index_capacity_cut_seconds_max[5m]), or an alert on "a cut held the keyspace write lock past the 2 ms query deadline recently" — reads the all-time worst, not the recent worst. One slow cut during a bootstrap flood or a soak pins the value forever and the metric is thereafter indistinguishable from a fleet that is still cutting badly. That is the opposite of what the cuts doc promises ("their count and duration are the write-lock hold time queries stall behind").

The other two are also already a histogram's _count/_sum pair by construction, and this crate has the histogram: LatencyHistogram (line 78) buckets 10 µs → 50 ms — the right range for an evict_oldest cut — and render() emits _bucket/_sum/_count. A pub cut_latency: LatencyHistogram on ServiceStats, observed where the three atomics are now, gives windowed quantiles, keeps _sum/_count, drops CutStats entirely, and renders with the same one-liner as the other two histograms:

+ &stats.cut_latency.render("radix_index_capacity_cut_duration_seconds")

It also makes the cut cost directly comparable, bucket for bucket, against radix_index_apply_duration_seconds — which already includes it, since enforce_capacity runs inside the timed apply.

pub struct CutStats {
pub count: std::sync::atomic::AtomicU64,
pub ns_total: std::sync::atomic::AtomicU64,
pub ns_max: std::sync::atomic::AtomicU64,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: the new counters are untested, and they're the direct assertion the two hysteresis tests currently have to make sideways.

Nothing in crates/radix_index/ reads cut_stats() except render_metrics, so count/ns_total/ns_max have no coverage — a future refactor that moves either fetch_add out of the > bound branch (or drops the instrumentation while keeping the accessor) leaves the metric silently flat with every test still green.

More usefully, capacity_cut_leaves_headroom_so_echoes_die (line 1827) infers "exactly one cut, then headroom" from entry_count() alone — it cannot distinguish "no truncation below the bound" from "truncated to the same size twice". The storm this crate was built around is precisely cuts ≈ applies, and that is now one counter away from being pinned:

engine.apply(&placement("w1", 3, 9)); // crosses 8 -> cut to 4
assert_eq!(engine.cut_stats().count.load(Relaxed), 1);
// ...after the two in-bound applies:
assert_eq!(engine.cut_stats().count.load(Relaxed), 1, "no cut below the bound");

Same in repeated_capacity_cuts_do_not_accumulate_state (line 1696): cuts <= rounds * 2 over 40 rounds would lock the docstring's "truncation is a rare event whose cost is amortized", which no current assertion covers.

@slin1237
slin1237 requested a review from njhill as a code owner September 15, 2026 14:09
@github-actions github-actions Bot added grpc gRPC client and router changes model-gateway Model gateway crate changes labels Sep 15, 2026
// stale local generation means every update we send is dead on
// arrival. Adoption is a new generation, so replay from zero
// (the resubscribe below starts at `last_seq`).
if let Some(adopted) = adopt_epoch(epoch, sent_wire(last_seq), ledger.known(&worker)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Important: epoch adoption is keyed by the worker, but with lanes the index acks the lane holder — so adoption never fires for a hybrid-model worker, and the failure it exists to prevent comes back permanently.

run_publisher_with_digest records acks under the ack's holder name (server.rs:340 sets holder: msg.holder.clone(), and convert_batch_lanes names those updates w1#0, w1#1, …), so the ledger fills with "w1#0" -> (epoch, seq) entries. Both adoption sites here look up ledger.known(&worker) with the bare "w1", which stays (0, 0) forever, so adopt_epoch returns None on every pass — line 560's mid-stream check too.

Failure: worker w1 is a hybrid model, so every event batch converts to w1#0/w1#1 only. The bridge runs a while and reaches epoch 3 (two ring-wrap gaps). The bridge process restarts; worker_loop starts at epoch = 1. The index still holds w1#0 at epoch 3, so apply_locked's epoch gate takes update.epoch < holder.epochDeduped on every update. Before lanes, ledger.known("w1") would have returned (3, n) and adopt_epoch would have bumped to 4; now nothing ever bumps, and both lanes of that worker are frozen at their pre-restart block set for the lifetime of the process — silently, since Deduped is a normal ack.

The lookup needs to be over the holder names this worker actually publishes. Either track the max (epoch, seq) the ledger holds across worker and every lane_holder(worker, g) in lanes.announced, or have EpochLedger::known accept the worker and fold its lanes (it already owns the map, and lane names are exactly worker + LANE_SEPARATOR + digits).

Comment thread crates/radix_index/src/bridge.rs Outdated
lanes
.announced
.get(&group)
.map_or(1, |info| info.block_size / block_size),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Important: map_or(1, …) guesses the unit count for a group the current process has not seen a Stored for, and a wrong guess is a silently lost eviction.

LaneBook is per worker_loop invocation (line 499), but the index keeps lane holders across a bridge restart (that is what epoch adoption is for). After the bridge restarts, the first Removed for a multi-unit group arrives before any Stored for that group, so lanes.announced misses and units falls back to 1 — the emitted key is the native block_hash, while the blocks in the index are under unit_key(native, 0, k) / unit_key(native, 1, k), which unit_key guarantees are different values for k > 1 (the test at line ~1100 asserts exactly that). Engine::remove skips keys it does not hold, so the removal is a no-op and the index keeps blocks the worker evicted: over-match, then a mis-route to a worker that has to re-prefill.

Line 352 has the same root cause in the other direction: Cleared fans out only to groups announced in this process, so a Cleared arriving before the first Stored of each lane leaves every pre-restart lane holder fully populated.

A guess is the wrong shape here — there is no unit count on KvBlocksRemoved to recover it from. Either hold removals for an un-announced group until its first Stored fixes units, or at minimum tracing::warn! instead of silently emitting a key that cannot match, so the divergence is visible rather than showing up later as an unexplained anti-entropy pull.

Comment thread crates/radix_index/src/bridge.rs Outdated
});
continue;
}
let unit_len = b.token_ids.len() / units as usize;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: the unit length is derived from the token count rather than from the keyspace block_size it has to match, so any disagreement between block_size and token_ids.len() silently produces content hashes no query can ever match.

units comes from blocks.first().block_size / block_size, but unit_len comes from this block's token_ids.len() / units. They only agree while every block in the event carries exactly lane_block tokens. When they don't:

  • token_ids.len() = 6, units = 2 (lane block 4, keyspace block 2) → unit_len = 3, so the unit contents are content_hash([t0,t1,t2]) and content_hash([t3,t4,t5]), while request_content_hashes hashes 2-token chunks. The lane stores blocks that match nothing, forever — no warning, and the lane looks healthy in every counter.
  • token_ids empty → unit_len.max(1) chunks an empty slice → zero blocks pushed for that engine block, so the next block's parent link points at a unit key that was never stored and the whole rest of the chain re-roots.

The keyspace block size is the thing the positions must tile, so chunk by it directly and refuse the block if the tokens don't come out even:

Suggested change
let unit_len = b.token_ids.len() / units as usize;
if b.token_ids.len() != (units * block_size) as usize {
tracing::warn!(
tokens = b.token_ids.len(),
units,
block_size,
"lane block token count does not tile the keyspace block; block skipped"
);
continue;
}
let unit_len = block_size as usize;

(stored_kind needs block_size threaded in as a parameter for this; the only caller already has it.)

if holder.dropped {
continue;
}
let matched_blocks = if first.start == 0 { first.end } else { 0 };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: dropping the depth == 0 filter changes the query contract for every consumer, not just lane-aware ones — and QueryOutcome::Empty no longer means what its doc says.

The old code skipped o.depth == 0, so every holder in Match.scores had at least one block matched from position 0. coverage emits a run wherever a holder covers any position of the query, so a holder whose only overlap is mid-chain now comes back with matched_blocks: 0. That is right for lanes, but it also fires for an ordinary event-fed worker that evicted its prefix and kept a mid-chain block — no lane, no cache_group, just normal eviction.

Downstream, client.rs:188 maps a non-empty scores to QueryOutcome::Scores and only an empty one to Empty ("the index answered with no overlap"). So a query where nothing has a usable prefix now returns Scores([...]) instead of Empty, and the ordering here puts the alphabetically-first zero-depth holder at the head. A gateway that reads Scores as "cache-hit candidates exist, take the top one" routes to a worker with no prefix at all rather than falling through to load-based placement. #2438 is where that lands, so it needs to filter on matched_blocks > 0 (or on intervals) rather than on the variant — worth saying so on QueryOutcome::Empty's doc here so the contract is written down before the consumer is built.

Separately on the hot path: lane_meta: holder.metadata.clone() is a fresh Vec per holder per query under the read lock, and now for more holders than before. The thread-local QUERY_SCRATCH exists precisely to keep this path allocation-free once warm; intervals has the same shape. Both are small, but they scale with the answer set that just grew.

out.push(HolderRun {
start,
end,
keys: (start..end).map(|p| pos_key[&(c, p)]).collect(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: pos_key[&(c, p)] turns a key-map/span-membership skew into a panic inside the Pull path, which is the one place the crate can least afford one.

runs trusts two independent structures to agree: state.chains + cd.spans say the holder covers (c, p), and state.keys is expected to have a key mapping there. place_block and remove do keep them paired today, so this should hold — but every other read in this file degrades instead of asserting (live() returns None, coverage does let Some(state) = … else { continue }). Here a single missing entry unwinds out of RadixTree::runsEngine::snapshot_holder → the Pull producer task, while the keyspace RwLock is held, which poisons it and takes the whole keyspace down for queries too — a bootstrap bug escalating into a serving outage on the replica that was healthy.

get + skip keeps the same behaviour when the invariant holds and degrades to a short run rather than a dead keyspace when it doesn't:

Suggested change
keys: (start..end).map(|p| pos_key[&(c, p)]).collect(),
keys: (start..end).filter_map(|p| pos_key.get(&(c, p)).copied()).collect(),

(with a debug_assert_eq!(keys.len(), (end - start) as usize) if you want the invariant still enforced in tests).

@slin1237
slin1237 force-pushed the fix/radix-tree-age-eviction branch from 434bec4 to a9126d8 Compare September 15, 2026 14:50
@slin1237
slin1237 changed the base branch from fix/radix-tree-age-eviction to feat/radix-tree-coverage September 15, 2026 14:50
@github-actions github-actions Bot removed grpc gRPC client and router changes model-gateway Model gateway crate changes labels Sep 15, 2026
@slin1237
slin1237 force-pushed the feat/radix-tree-coverage branch from 54e94b0 to 2f6f7ac Compare September 15, 2026 14:56
Comment on lines +130 to +134
from one `coverage` walk. The reuse rules (full attention = coverage from
0, window = `W-1` tokens before the candidate, checkpoint = a block end at
the candidate, candidates aligned to the lanes' blocks) live in the
gateway (`model_gateway/src/policies/reuse.rs`), where the engine's
semantics belong.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: model_gateway/src/policies/reuse.rs does not exist in this repo, so this is a dangling pointer for anyone who follows it.

model_gateway/src/policies/ currently holds bucket.rs, cache_aware.rs, cache_namespace.rs, consistent_hashing.rs, dp_min_token.rs, factory.rs, least_load.rs, manual.rs, mod.rs, passthrough.rs, power_of_two.rs, prefix_hash.rs, random.rs, registry.rs, round_robin.rs, utils.rs — no reuse.rs. The PR description is explicit that the gateway integration is #2438 ("No gateway code changes here"), so this section documents a path that only lands in a later PR. Pointing at the PR instead survives whatever the file ends up being called.

Everything else in the new section checks out against the code: unit_key(native, unit, units) (src/bridge.rs:241), the non-tiling refusal (src/bridge.rs:294), HolderScore.intervals/matched_blocks (proto/radix_index.proto:155-160), the lifecycle fan-out over LANE_SEPARATOR (src/engine.rs:515-521), and the per-run path-prefixed snapshot (src/engine.rs:1287-1340).

Suggested change
from one `coverage` walk. The reuse rules (full attention = coverage from
0, window = `W-1` tokens before the candidate, checkpoint = a block end at
the candidate, candidates aligned to the lanes' blocks) live in the
gateway (`model_gateway/src/policies/reuse.rs`), where the engine's
semantics belong.
- Answers carry every covered run of the query per holder
(`HolderScore.intervals`) next to the contiguous depth (`matched_blocks`),
from one `coverage` walk. The reuse rules (full attention = coverage from
0, window = `W-1` tokens before the candidate, checkpoint = a block end at
the candidate, candidates aligned to the lanes' blocks) live in the
gateway's routing policy (#2438), where the engine's semantics belong.

Comment on lines +30 to +32
is the path for engines/modes with no KV event stream. Inferred
state is bounded by idle TTL + per-holder capacity with tail-first
(prefix-closed) eviction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: "tail-first (prefix-closed) eviction" describes the first version of the capacity cut, which this stack replaced — and it is the version the PR description says leaked memory.

Engine no longer calls truncate_tail at all; the only capacity cut is tree.evict_oldest(holder.id, capacity) (src/engine.rs:862, documented at :846 as "The cut is recency-ordered (evict_oldest)"). Per the PR body, depth-ordered truncation "dropped the freshest long-prompt tails and never freed a chain slot — the 30-minute soak measured replica RSS 120 → 460 MiB". An operator who reads this line will reason about capacity behavior (and about which prompts survive a cut) using the discarded semantics — exactly backwards, since whole-chain recency eviction drops the oldest whole chains, not the newest tails.

Suggested change
is the path for engines/modes with no KV event stream. Inferred
state is bounded by idle TTL + per-holder capacity with tail-first
(prefix-closed) eviction.
(`seq=0`, content-idempotent). No engine cooperation needed — this
is the path for engines/modes with no KV event stream. Inferred
state is bounded by idle TTL + per-holder capacity, cut
recency-ordered (least-recently-published whole chains first).

Line 73's --default-capacity-blocks row says "truncates" for the same reason and is worth the same edit (CodeRabbit flagged that row for the threshold; the ordering wording is a separate staleness).

…query path

overlap answers one number per holder: the consecutive depth from
position 0. That is the shape of a contiguous prefix cache and only that.
An engine whose cache is several independently evicted position sets
(full attention, a sliding window that frees its old blocks, recurrent
state kept at checkpoints) stores fine in the tree, whose per-holder
coverage is spans at arbitrary positions with holes allowed, and then
reads back wrong: coverage that starts past position 0 scores zero.

Add coverage(query, scratch, out): the same trie descent as overlap
(shared as match_path), then per span x holder instead of the active-set
intersection, emitting maximal [start, end) runs per holder in path
order, sorted by (holder, start). Scratch-backed like overlap: a
per-slot open-run index reset in O(touched), no per-query heap once
warm. overlap is unchanged in behaviour and cost.

Tests: holes, mid-path runs and divergence against overlap's answer;
scratch reuse across queries and retired holders; a randomized churn
(shared prefixes, random removes) checking every holder's runs against
the per-position key map.

Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
One index the gateways query and feed over gRPC so every gateway sees
the same fleet-wide view of which worker holds which prefix, instead of
each gateway rebuilding its own from the KV-event streams.

Engine: one keyspace per model x symbol kind x block size, each a
RadixTree behind its own lock; placements are unsequenced and idempotent
(shared-lock dedup fast path, optional {tip,len} digests), worker events
are sequenced with epochs and a single owner per worker. Replicas copy
rather than agree: an apply is relayed only when it changed state, new
replicas bootstrap by Pull, lifecycle add/drop plus a silence backstop
retire dead workers. Queries carry a hard deadline and fall back to
local routing.

Ships the service and bridge binaries, the gateway-side client, a
versioned wire-hash scheme pinned against kv_index, bench/loadbench
tools, the StatefulSet manifest, and live two-replica, client fault-path
and lifecycle/digest integration tests. Published as tier 3 (depends on
smg-radix-tree and smg-grpc-client). No gateway code changes.

Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
A worker's cache is not always one contiguous prefix. Hybrid models keep
several independently evicted position sets per worker (full attention,
a sliding window that frees its old blocks, recurrent state saved at
checkpoints), and the reusable prefix is the largest position every set
accepts at that position. The index stored such coverage fine (spans at
arbitrary positions) but folded every set a worker emitted into one
holder and answered one number per holder, the depth from position 0.

The index now stores lanes and answers intervals; it still knows nothing
about attention kinds. A lane is one independently evicted set,
published as its own holder named worker#lane, with the publisher's
opaque description on Added.metadata, kept on the holder, carried by
snapshots and echoed on every answer. Answers carry every covered run of
the query path per holder (HolderScore.intervals) next to the
contiguous depth, from one coverage walk. Lifecycle control addressed to
a worker fans out to its lanes.

The vLLM bridge splits batches by the engine's cache group, announces
each lane once with kind, window and block size, and slices a lane whose
engine block spans k keyspace units into k unit blocks with derived
removal keys, so lanes with different block sizes share one keyspace.
The Python converter carries group_idx, kv_cache_spec_kind and the
window on the KV event wire (additive fields; legacy consumers ignore
them).

Two latent bugs surfaced by the lane tests and fixed here: a snapshot
of a holder whose coverage starts past position 0 was shipped as a
parent-less Stored and landed at position 0 on a new lineage on the
puller (silent positional divergence after any bootstrap or
anti-entropy pull of an event-fed holder with mid-chain removals);
snapshots are now per run, path-prefixed with placeholder blocks that
the same snapshot removes. And the replica digest hashed contents only,
so that divergence was invisible to anti-entropy; it is position-bound
now.

Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
@slin1237
slin1237 deleted the branch fix/radix-tree-age-eviction September 15, 2026 15:40
@slin1237 slin1237 closed this Sep 15, 2026
@slin1237 slin1237 reopened this Sep 15, 2026
@slin1237
slin1237 changed the base branch from feat/radix-tree-coverage to fix/radix-tree-age-eviction September 15, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD configuration changes dependencies Dependency updates documentation Improvements or additions to documentation tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant