Repository navigation
feat(kv-router): expose prefix hashes, policy class, and worker capacity to selection policies - #15449
feat(kv-router): expose prefix hashes, policy class, and worker capacity to selection policies#15449PeaBrane wants to merge 3 commits into
Conversation
…policies Worker-selection plugins could not key on a shared prompt prefix, see the class a request classifier assigned, or read a worker's advertised capacity. - WorkerSelectionContext::prefix_hashes() borrows the request's chained per-block tracking hashes. - WorkerSelectionContext::policy_class() returns the requested class. A classifier's override is now recorded on the request before selection. - WorkerSelectionContext::worker_capacity() looks up a worker's advertised total_kv_blocks and max_num_batched_tokens, mapping the zero some backends advertise for "unknown" to None. Capacity is a lookup rather than a WorkerInputs group: carrying it alongside every candidate row slowed the default policy by about 2% at 1,024 workers, although the default never reads it. Signed-off-by: PeaBrane <yanrpei@gmail.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Repository guideline files applied to this review (6)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review. WalkthroughWorker-selection contexts now expose request prefix hashes, policy class, and worker capacity. Selector materialization supplies capacity lookups from worker configuration, and classifier validation stores the selected policy class on the request. ChangesWorker Selection Context
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to No merge-blocking issue is established for these read-only policy inputs. The change appears mergeable subject to normal checks. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides detailed scope, implementation guidance, validation results, and reviewer guidance. However, it omits the required Related Issues section and does not confirm whether the pull request has no related issue. Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 7 files. (1 skipped: 1 unsupported.)
Comment |
…acity Address review on the policy-input API: - Bound the capacity lookup by Sync so WorkerSelectionContext stays Send and Sync for plugins that score candidates in parallel. Every host config is already Sync. - Drop WorkerCapacityInput::max_num_batched_tokens until a policy needs it. - Document that the standalone selection service passes hash-only prefix hashes through unchecked and that policy_class is not validated. - Assert the contract test's scorer actually runs. Signed-off-by: PeaBrane <yanrpei@gmail.com>
⏹️ Dynamo PR CI cancelled — run 37086387586 (attempt 1) on
|
| Other | Jobs |
|---|---|
| dynamo-runtime | ⏹️ 1 ✅ 7 |
Failure details
No jobs failed. The 1 cancelled job, dynamo-runtime / rust-gpu, was not a fail-fast or manual cancel: it hit the job's 30-minute execution limit (The job has exceeded the maximum execution time of 30m0s) during step Run Rust checks (block-manager + media-ffmpeg + MM routing + integration tests). When it was stopped, it was still running the kvbm_physical unit tests and every test so far had passed. Rerunning the failed jobs is likely enough. If it times out again, the Rust GPU test step is too slow for its 30-minute limit.
For agents
{"pr": 15449, "run_id": 37086387586, "run_attempt": 1, "head_sha": "2c41b73ce6f0a97e69afddc9f39cd7140ecfe935",
"failures": [{"job": "dynamo-runtime / rust-gpu", "job_id": 111103285326, "conclusion": "cancelled", "failed_step": "Run Rust checks (block-manager + media-ffmpeg + MM routing + integration tests)", "signature": "job exceeded maximum execution time of 30m0s", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/111103285326/logs"}]}Posted automatically by Devin for run 37086387586. Updated on every full-CI run of this PR.
Signed-off-by: PeaBrane <yanrpei@gmail.com>
| | `session_context()` | Optional session metadata described below | | ||
| | `prefix_hashes()` | Optional chained hashes of the prompt's complete KV blocks; entry `i` identifies blocks `0..=i`, so equal entries mean an exactly shared prefix. `None` when the frontend does not track active blocks, as in disaggregated prefill pools; unique to the request when the host does not assume KV reuse, as in disaggregated decode pools. The standalone selection service passes a hash-only request's precomputed hashes through unchecked | | ||
| | `policy_class()` | Optional policy class: the caller's value, replaced by a request classifier's override. A caller's value is not validated against the policy profile | | ||
| | `worker_capacity(worker)` | Capacity advertised in a worker's runtime config: `total_kv_blocks()` per data-parallel rank, `None` when not advertised or advertised as zero. `None` for a worker the host does not know. Each call looks the worker up, so policies that never call it pay nothing | |
There was a problem hiding this comment.
I think there should be a cleaner way to access the MDC of each worker if a cost function wants it
Summary
Adds three read-only inputs to the public worker-selection plugin API
(
plugins::worker_selection). Porting recently published routing policies onto the plugin APIshowed they needed inputs a plugin could not get. The stacked follow-up PR ports those policies.
WorkerSelectionContext::prefix_hashes()borrows the prompt's chained per-block trackinghashes. Entry
iidentifies blocks0..=i, so a policy can key on a shared prefix for hot-spotdetection, consistent hashing, or two-choice hashing.
Nonewhere the host tracks no active blocks, such as disaggregated prefill pools.decode pools.
WorkerSelectionContext::policy_class()returns the request's policy class. A requestclassifier's
set_policy_classoverride is now recorded on the request after validation, soselection sees the classifier's decision rather than only the caller's value. Nothing
downstream of classification read the field before this change.
WorkerSelectionContext::worker_capacity(worker)looks up the worker's advertisedtotal_kv_blocksandmax_num_batched_tokens. It maps the zero that some backends advertisefor "unknown" to
None.worker_capacityis a lookup into the worker table the host already holds, not a newWorkerInputsgroup. A first version carried capacity beside every candidate row and slowedthe default policy by about 2% at 1,024 workers, even though the default never reads capacity.
The lookup leaves candidate rows, views, and picker columns unchanged.
Reviewer guide:
plugins/worker_selection/context.rsandinputs.rs(WorkerCapacityInput).scheduling/selector/mod.rs.scheduling/queue.rs.scheduling/selector/policy.rs.custom-worker-selection.mdx.Validation
Default selection cost is unchanged. Criterion benches compared
origin/mainagainst thisPR, interleaved as baseline, candidate, baseline, candidate on one pinned core. The builtin
default policy (
default_selection/t0/plugin), baseline vs this PR, per selection:origin/mainkv-router's composed-policy benches (
custom_worker_selection,unused_preferred_taint_metadata)are equal or faster.
Replay routing is byte-identical. Offline replay canonical reports match
origin/mainbytefor byte, in two fresh processes each, across four configurations: vLLM and SGLang semantics,
each aggregated and disaggregated. Each ran the first 5,000 Mooncake trace rows on the
replay-benchseeded router. The four digests are the same as the baseline's.and 10 SGLang retractions.
Tests:
request_facts_and_worker_capacity_reach_policy_componentsexercises allthree accessors through a scorer and a picker, including zero-as-unknown capacity and an
unknown worker.
classifier_overrides_are_validated_before_enqueuenow asserts the write-back.Suites and lint:
cargo test -p dynamo-kv-router --libpasses (1,032 tests). This PR onits own also passes the builtin and mocker lib tests. Workspace clippy is clean.
🤖 Generated with Claude Code
Summary by CodeRabbit