Repository navigation
feat(router): support single-source fetch continuations - #17657
Conversation
There was a problem hiding this comment.
Questions for the author:
- lib/kv-router/src/indexer/lower_tier.rs: With
router_fetch_continuationenabled, each inference lookup adds a second full prompt-chain walk and builds per-request source vectors plus anFxHashMapwhose entries scale with distinct owners. Please provide the expected owner/cardinality calculation and lookup-latency or throughput measurements for representative long-prompt, continuation-enabled workloads so the inference-path cost can be assessed.
|
Following up on the owner/cardinality and lookup-cost question: I measured the current head ( Cardinality: destination count T does not bound source count S. S is the union of worker/rank residency owners and advertised exact cache owners observed along the chain; worker capability/topology filtering happens later. For B reachable blocks, discovery is O(B + sum of owner counts across changed owner-set runs). Stable identical Multi sets coalesce to roughly O(B+S); changing sets can approach O(BS). Staggered suffixes can cause O(S²) owner visits with only S final intervals. Fragmentation can produce I=O(BS) intervals. Summary construction sorts I references (O(I log I) upper bound), while each target lookup is O(log distinct starts), not a scan over S. Measured cost: microseconds below are medians of six per-process percentiles. The combined component includes lower-tier lookup, the current candidate clone/drop handoff, summary construction and source choice for every target. Device matching, full worker scoring, queues, transport and inference are excluded. Blocks represent 16 tokens; these are synthetic inventories, not a production workload distribution.
Single/pair-owner 32k shared-suffix cases took 59.72/83.00 us p50: the fingerprint coalescing applies to Multi entries, so low source cardinality does not remove the per-block work. The 32k fragmented fixture emits 3,072 intervals and requests 550 KiB of allocation traffic per combined operation; the 128k fixture emits 12,288 and requests 2,184 KiB. Those are cumulative allocation/reallocation bytes, not peak heap. The run used a Xeon 6975P-C CPU VM, one pinned physical core, identical release build settings and glibc, separate allocation instrumentation, interleaved fresh processes, and six-per-label A/A first. Default-off/base p50 ratios were 0.978–1.024 with every 95% interval including 1.0; A/A intervals spanned roughly 0.963–1.044. Enabled-cost differences clear that noise floor. All 34 processes × 15 inventories passed independent source/interval oracles, with base/off discrete outputs identical. Raw trials, hashes, allocation counts, and confidence intervals are included. This measures router overhead, not sustained serving throughput or net fetch benefit. The fully-local control exposes useful follow-up work: prune intervals using actual destination frontiers, filter before sorting, and move candidates instead of cloning immediately before dropping their owner. None of those improvements is claimed in these measurements. The flag remains default-off. |
|
Follow-up to the cardinality/cost review: the updated implementation makes continuation standard fetch-hint behavior and removes the new flag and all its plumbing. The previous comment's measurements remain specific to The follow-up report and raw trials compare that enabled implementation with the optimized one on the same 15 inventories. Candidate handoffs now move instead of clone; summary filtering precedes sorting; suffix discovery is skipped without positive partial device+local-host frontiers and emits only intervals usable at an original frontier. Root-prefix fetches remain available to cold workers, including workers absent from the hit map. No new destination-filter plumbing is needed. Combined component p50 / p99 (µs, old → new):
These reductions clear the A/A noise. Other cells moved roughly −1.3% to +3.4%; the single-owner case increased 60.26→62.29 µs, so this is not uniformly faster. All 420 process/fixture validations passed with identical source choices. Six fresh processes per timed arm, interleaved; allocation instrumentation ran separately. The unchanged source-run traversal can still approach O(B*S), and source count is not bounded by destination count. The ordering is documented and regression-tested: L1 → contiguous local L2 → one remote L2 source. We do not resume detached local L2 after a remote fetch or plan multiple remote sources. Real-KVCR CPU validation also passed 32 byte-checked fetches plus a rejected wrong-source control. No serving-throughput or net fetch-benefit claim is made. |
jthomson04
left a comment
There was a problem hiding this comment.
Perf-focused review. The selection logic looks right: cumulative top two per start depth, with expired intervals dropped by the prefix check. Moving the candidates instead of cloning them is a nice win on its own. The main problem is the cost of the second lower-tier walk now that it is on by default; measurements are on lower_tier_indexers.rs.
There was a problem hiding this comment.
Questions for the author:
- lib/kv-router/src/indexer/lower_tier.rs: The published mooncake_bench regression predates the current fused-walk implementation in b330817, while the new default hint path still computes owner fingerprints per traversed block and retains active sources as owner sets change. Please rerun the same representative Lease/Book long-prompt workload against the reviewed head and provide lower-tier lookup p50/p99 (or equivalent throughput data) so the current inference-path cost can be assessed.
|
/ok to test 3677922 |
✅ Dynamo PR CI passed — run 41546660668 (attempt 1) on
|
Signed-off-by: Yan Ru Pei <yanrpei@gmail.com>
Signed-off-by: Yan Ru Pei <yanrpei@gmail.com>
Signed-off-by: Yan Ru Pei <yanrpei@gmail.com>
Signed-off-by: Yan Ru Pei <yanrpei@gmail.com>
Signed-off-by: Yan Ru Pei <yanrpei@gmail.com>
Signed-off-by: Yan Ru Pei <yanrpei@gmail.com>
3677922 to
4b943ae
Compare
|
/ok to test 4b943ae |
Resolve the conflicts with #17657, which moves the retained KV transfer chain out of TieredMatchDetails with its own into_kv_transfer_candidates: drop take_kv_transfer_candidates and its test, make into_kv_transfer_candidates public, and call it from offline replay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Yongming Ding <yongmingd@nvidia.com>
Absorbs the 95 upstream commits since 464ff29. Each upstream change lands in the crate that owns that code at the stack head; the stack's moves stay intact. Router-touching PRs and where they landed: - ai-dynamo#17712 host-path routing modes for worker-selection plugins: &self + Send + Sync plugin traits and WorkerInputs::ACTIVE_REQUESTS (deprecated OCCUPANCY alias) in dynamo-worker-selection and dynamo-router-protocol; HostedWorkerPicker and HostedSelectionRequest in dynamo-worker-selection (hosted.rs), reporting SelectionFailure, which the kernel converts into KvSchedulerError; the InstanceSelector adapter in lib/llm (HostedInstanceSelector); the compat test's hosted coverage in kv-router's prelude test. - ai-dynamo#17713 random/round-robin/least-loaded as builtin plugins: router_modes in dynamo-router-policies (renamed from dynamo-custom-policy-builtin) importing the prelude; RouterPluginRegistry::resolve_type in kv-router; the frontend's hosted path through dynamo_worker_selection::host. - ai-dynamo#17899 1.7.0 removals: taken whole; the alias and the setter were gone at the head; power-of-two never arrives. - ai-dynamo#17657 single-source fetch continuations: KvTransferSourceInterval and source_intervals in dynamo-kv-indexer, the bucketed summary in dynamo-kv-hints, the interval filter in the kernel's resolver; tests in the kernel. - ai-dynamo#17856 KVCR hints in offline replay: mocker and lib/llm bodies; supports_kv_transfer_chain_retention on the flat config; its indexer-vs-config test in kv-router's core host tests. - ai-dynamo#17794 vLLM partial-tail filter and ai-dynamo#17126 SGLang engine-owned multimodal pads: dynamo-kv-events zmq_wire (mm_exclusion.rs); the codec-plus-indexer tests in dynamo-kv-indexer; the engine_owned_mm_identity worker flag through the kernel's WorkerRequest, kv-router's indexer registry and run config, and the bindings. - ai-dynamo#17653 SGLang 0.5.23: pad constant docs in dynamo-multimodal. - ai-dynamo#15333, ai-dynamo#17874, ai-dynamo#17484, ai-dynamo#17688, ai-dynamo#17237, ai-dynamo#17894, ai-dynamo#17958: host-side changes merged in place (bindings, lib/llm, CI). Lockfiles regenerated; no shim reintroduced. Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
Overview
Allow a fetch hint to extend a worker's existing GPU/host-pinned prefix from one source holding only the missing continuation. A target holding blocks 1–10 can fetch 11–15 from a source without blocks 1–10. This is standard behavior for existing hint-capable deployments, with no new configuration, CLI option, environment variable, or wire field.
Key Changes
Approximately 316 added non-test, non-comment Rust lines; no new production dependencies.
lower_tier.rs/lower_tier_indexers.rs: discover exact fetch sources when an original local worker walk stops, using the edge already loaded by that walk. Active sources only shrink; a source can rejoin only at another destination's independent original frontier. There is no second root-to-tail scan. Single/Pair owner IDs and the existing Multi fingerprint skip repeated membership work; frontier handoffs move active maps and intervals.kv_hints/sources.rs: rank sources by start depth and choose at the destination's actual local frontier. Preserve topology and attachment exclusions, source/score consistency, and conservative prefill accounting. Filter intervals against both minimum and maximum local depths before sorting; move retained candidates into selection and refresh.Validation
Behavioral tests cover gaps, eviction, dump/replay, cold/host-only/partial/full frontiers, topology, attachment exclusion, and independent owner-inventory oracles. The hash-consistent differential soak passes 20,000 cases with its oracle unchanged. The router suite passes 1,396 tests (3 ignored), with three builtin fetch-policy tests also passing. The soak was rerun after the map-move optimization; subsequent changes only remove redundant tests and a comment.
The separate CPU validation harness passed 32 real-KVCR suffix fetches with byte-for-byte delivery checks, rejected a wrong source, and verified partial success after a stale local prefix. It uses CPU buffers, a byte-copy NIXL adapter and loopback ZMQ. It does not run an engine scheduler or validate native NIXL, GPU/RDMA, inference, or serving performance. The receiver must use actual fetch outcomes and recompute any remaining gap; a hint is not a pin.
The completed Mooncake comparison and all trials use main
670019d948versus the fused implementation on the same source: six fresh processes per arm, after six per A/A label. Lower-tier lookup p50 is 1.162→1.209 ms (+4.1%, 95% interval +0.3% to +6.2%); p99 is 3.835→3.692 ms (−3.7%, interval −12.3% to +2.6%). All 26 processes, including scouts and A/A, passed generator/keep-up and final membership checks.This is directional x86 evidence on a public AgentX-shaped replay, not the reviewer's exact trace or Grace reproduction. A/A ratio intervals were 0.852–1.111 for p50 and 0.781–1.265 for p99: the observed p50 increase is smaller than that noise range, and the p99 decrease is inconclusive. Queueing is retained in the report: scheduled-to-finished p99 rose 410→443 ms and final drain 35→58 ms. Both arms handle the fixed offered load; this is not a capacity-knee or serving-throughput result. Earlier component results measured the removed global scan and do not describe this revision.
Issues and Ongoing Work
Pull Request Checklist
/pr-checklistskill.