Repository navigation
Conversation
…onal indexer goes #2814 shipped the chain index beside the positional indexer behind `--kv-index`, the positional one the default until the chain index had passed its gateway soak. The soak is in: the gateway runs the chain index alone and the positional path is deleted. Nothing names the positional indexer any more. The `--kv-index` flag, the `kv_index` config field and `KvIndexKind` (with its `run` alias) go, in the CLI, the YAML and the Python launcher; the gateway always builds the chain index and `KvEventMonitor::new()` takes nothing. The monitor's kind, jump-size and prune machinery goes with them. `--kv-indexer-ttl-secs` and `--kv-indexer-max-entries`, public since #2095, stay as deprecated no-ops with a startup warning so that existing YAML loads. `crates/kv_index/src/event_tree.rs` goes, with `PositionalIndexer`, `PruneStats`, `WorkerBlockMap` and the `ContentSeq` trait from the crate's surface, its exactness harness `tests/exactness_positional.rs` with it (the chain index's own covers the same corpora), and the positional arms of the benches: mooncake_replay's backend and `--jump-size`, match_insert's group, throughput_bench now on `ShardedChainIndex`. The hash vocabulary it hosted the tests of (`compute_content_hash` against the streaming hasher, request chunking) moves to `src/block.rs` with them; the radix-tree differential oracle and the decision bench run on the chain index. The chain path itself is untouched beyond wiring. The crate's public surface shrinks, so it is 2.0.0 and the workspace pin follows. Reverting this commit restores the positional path as #2814 shipped it. BREAKING CHANGE: the `--kv-index` flag, the `kv_index` config field and the `KvIndexKind` type are removed; the gateway always runs the chain index (the former `--kv-index chain`). `--kv-indexer-ttl-secs` and `--kv-indexer-max-entries` are deprecated no-ops that warn at startup; their config fields stay so that existing YAML loads. Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
| /// Error returned by [`ChainIndex::intern_worker`](crate::ChainIndex::intern_worker) when the u32 worker-id | ||
| /// space is exhausted. Ids are assigned monotonically and never recycled, so this | ||
| /// can only be reached after `u32::MAX + 1` distinct worker URLs have been interned | ||
| /// over the indexer's lifetime. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub struct WorkerIdExhausted; | ||
|
|
||
| impl fmt::Display for WorkerIdExhausted { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| write!(f, "worker id space exhausted (u32::MAX workers interned)") | ||
| } |
There was a problem hiding this comment.
🟡 Nit: This doc was carried over from event_tree.rs, where it described PositionalIndexer::intern_worker. It now links to ChainIndex::intern_worker, and it doesn't hold there. The chain index recycles ids (registry.free.pop() / release_worker, chain_index.rs:444/479) and refuses a new worker once names.len() >= max_workers. The gateway sets that limit to CHAIN_INDEX_MAX_WORKERS = 1024 per model (kv_index_backend.rs:28).
This PR makes the chain index the only path, so 1,024 workers streaming events for one model at the same time is now a hard limit for every cache-aware deployment. The positional index had no practical limit. When an operator crosses it, subscription_loop logs error = "worker id space exhausted (u32::MAX workers interned)" (subscription.rs:166). That points them at a u32 overflow when the cause is the per-index slot limit.
Suggest rewording both the doc and the Display text to describe the slot limit, e.g.:
/// Error returned by [`ChainIndex::intern_worker`](crate::ChainIndex::intern_worker) when every
/// worker slot of the index is taken (`max_workers`, 1,024 per model in the gateway). Ids are
/// handed back when a worker is removed, so this bounds the workers interned at once, not the
/// churn over the index's lifetime.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct WorkerIdExhausted;
impl fmt::Display for WorkerIdExhausted {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "every worker slot of the index is taken")
}
}It may also be worth putting the new per-model limit in the PR description or release notes, since the soak only covered 8 and 128 workers.
Description
#2814 shipped the chain index beside the positional indexer behind
--kv-index, the positional one left as the default so that a revert stayed one flag away while the chain index ran its gateway soak. The soak is in, so the chain index becomes the only KV index and the positional path is deleted. This pull request is the revert unit: reverting it restores the positional path exactly as #2814 shipped it. One commit from the removal series at 2ef056d9 on main fad0f55, heads in the Provenance line at the end.Problem
Since #2814 the gateway has carried two KV indexes with one switch between them.
--kv-indexer-ttl-secsand--kv-indexer-max-entriesprune bounds applied to the positional indexer only.Solution
The chain index is the only index behind the gateway, and the switch goes with the code it selected.
--kv-index, thekv_indexconfig field,KvIndexKindand itsrunalias are removed from the CLI, the YAML and the Python launcher.KvEventMonitor::new()takes nothing, and the monitor's kind, jump-size and prune machinery goes.crates/kv_index/src/event_tree.rsat 3,245 lines, goes withPositionalIndexer,PruneStats,WorkerBlockMapand theContentSeqtrait.tests/exactness_positional.rsgoes too, the chain index's own covering the same corpora.--kv-indexer-ttl-secsand--kv-indexer-max-entries, public since feat(kv_index): bound the positional indexer with TTL and capacity pruning #2095, stay as deprecated no-ops that warn at startup, their config fields kept so existing YAML loads.block.rswith its tests.--jump-size, match_insert's group, throughput_bench now onShardedChainIndex.chain_index.rs, 4 insharded.rs, 2 inlane_map.rs.Changes
42 files against main fad0f55, 670 insertions and 4,931 deletions, by area.
crates/kv_index, 15 files: the positional indexer and its harness deleted, the hash vocabulary moved, the benches' positional arms gone, the crate at 2.0.0.model_gateway, 16 files: the switch, the backend enum, the monitor's kind and prune machinery and the positional path's metrics removed, the prune flags deprecated.crates/radix_treeand its benchmark workflow, 5 files: the differential oracle and the bench on the chain index.bindings/python, 4 files: thekv_indexargument and the launcher flag removed.Cargo.tomlandCargo.lock: the crate's version pin.Test Plan
-D warningsover the workspace's targets and the two CI feature configurations without OpenCV and nixl.block.rsadds 6.model_gateway/src/worker/kv_index_backend/exactness.rsandcrates/kv_index/tests/exactness_chain.rs.Provenance
perf/kv-index-chain-onlyat 19754b0, one commit from the removal series head 2ef056d9 on main fad0f55 (four commits behind main 833f798, which touch no shared file and auto-merge clean, not chased), the removed code being the positional path #2814 merged there.