Repository navigation
feat(router): make worker-selection plugins support host-path routing modes - #17712
Conversation
… on the hosted path Plugins can now be shared across request threads without a lock, and a picker-only policy can run outside the KV scheduler through a new hosted entry point. This is the foundation for running the non-KV router modes as ordinary plugins. - WorkerFilter::keep, WorkerScorer::score and WorkerPicker::pick take &self, and the traits require Send + Sync. The default policy keeps its per-call state in a local and its sampling scratch in a thread-local buffer, so selection still allocates nothing. - WorkerSelectionPolicy::into_hosted_picker returns a HostedWorkerPicker for a picker-only policy that declares at most ACTIVE_REQUESTS and has no runtime config; HostedWorkerPicker::select picks over a lazy view of the host's worker IDs with no candidate rows and no lock. - WorkerInputs::ACTIVE_REQUESTS replaces OCCUPANCY (deprecated alias until v1.9), with WorkerInputView::active_requests(row), len() and is_empty(). - HostedSelectionRequest carries a hosted selection's request metadata. BREAKING CHANGE: worker-selection plugin methods take &self instead of &mut self, and the plugin traits require Send + Sync. A plugin that mutates state between calls must use atomics or a lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Karen Chung <karenc@nvidia.com>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Shared callback contract and implementations lib/kv-router/src/plugins/worker_selection.rs, lib/kv-router/src/scheduling/selector/policy.rs, lib/router-plugins/builtin/src/default/*, lib/router-plugins/builtin/tests/*, examples/router/custom-policy-example/*, lib/kv-router/benches/*, lib/llm/src/kv_router.rs, deploy/inference-gateway/ext-proc/src/selector.rs, docs/fern/pages/developer-guide/knowledge-base/modular-components/router/custom-worker-selection.mdx |
Callback traits require Send + Sync, and callback methods take &self. Implementations and test policies are updated. DefaultPicker uses thread-local scratch buffers, and DefaultScorer passes prepared request data through scoring calls. |
Hosted picker and input view lib/kv-router/src/plugins/worker_selection/inputs.rs, lib/kv-router/src/scheduling/selector/*, lib/kv-router/src/scheduling/types.rs, lib/kv-router/tests/plugin_api_compat.rs |
WorkerInputView supports hosted worker IDs and active-request lookups. Eligible picker-only policies can become HostedWorkerPicker instances. Tests cover input access, selection results, and error cases. |
Active-request routing and migration lib/llm/src/kv_router/routing_host/*, docs/fern/pages/developer-guide/knowledge-base/modular-components/router/custom-worker-selection.mdx, docs/fern/pages/reference/general/releases/deprecations.mdx |
Built-in routing uses ACTIVE_REQUESTS where it previously used OCCUPANCY. The guide and release notes document ACTIVE_REQUESTS and the deprecated OCCUPANCY alias. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Merge Risk: 
Low · up to a3d97
-
A custom picker that selects by worker capacity could choose a different worker when run through hosted selection. This is a bounded compatibility risk to address before adopting that path for such pickers.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "feat(router): make worker-selection plugins support host-path routing modes" matches the main change. The PR makes worker-selection plugins shareable through &self receivers and Send + Sync bounds, and adds a hosted path (HostedWorkerPicker, into_hosted_picker) that runs picker-only policies without the KV scheduler. The title names the hosted routing path, but it does not mention the breaking &self/Send + Sync change or the ACTIVE_REQUESTS rename, which are also central to the changeset. That makes the title partially specific, but it is still related to the main change.
Full details: Description check
-
Explanation
The description includes all required sections: Overview, Key Changes, Validation, Issues and Ongoing Work, and the pull request checklist. It clearly explains the implementation, breaking API changes, validation coverage, and linked work. The checklist item remains unchecked, but the description is otherwise complete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 15.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 70 functions across 37 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
80913be to
8a62d01
Compare
Signed-off-by: Karen Chung <karenc@nvidia.com>
Signed-off-by: Karen Chung <karenc@nvidia.com>
Signed-off-by: Karen Chung <karenc@nvidia.com>
✅ Dynamo PR CI passed — run 41687980363 (attempt 1) on
|
ishandhanani
left a comment
There was a problem hiding this comment.
Read the full diff, ran cargo clippy -D warnings (kv-router, router-policies, routing-algorithms, runtime) and the kv-router / router-policies test suites at the stack head (17713 head, which carries this PR). All green. No correctness defects found; the hosted path, the &self change and the ACTIVE_REQUESTS wiring hold up. Comments below are on cost and API surface on the new hosted path.
Store the hosted picker as the policy's Box instead of converting it to an Arc nothing clones, and drop HostedSelectionRequest's three builders and its block_size argument, which no host calls. Hosted pickers see the same values as before: the prompt length, and the KV router's defaults for the rest. Signed-off-by: Karen Chung <karenc@nvidia.com>
…worker ID WorkerSelectionContext now holds the request values it exposes instead of borrowing the whole SchedulingRequest, so HostedSelectionRequest carries only the prompt length and SchedulingRequest::hosted() is gone. Hosted select returns the chosen WorkerId rather than a WorkerSelectionResult whose cache and load fields were always zero. Signed-off-by: Karen Chung <karenc@nvidia.com>
ishandhanani
left a comment
There was a problem hiding this comment.
Round 1 AI review
Automated review (round 1 of up to 2 before a human review). Findings may be wrong; reply on a thread to dispute one.
This PR changes the worker-selection plugin callbacks (keep, score, pick) to take &self and requires the traits to be Send + Sync. It adds HostedWorkerPicker, so a picker-only policy can run without the KV scheduler, and replaces OCCUPANCY with ACTIVE_REQUESTS, keeping OCCUPANCY as a deprecated alias. I traced into_hosted_picker eligibility, the lazy WorkerInputView::hosted rows, the KV-path active_requests closure, the default picker's thread-local buffers, the default scorer's per-call PreparedRequest and the WorkerSelectionContext field copy, and found no correctness defect. The KV path still projects loads before every selection, so ACTIVE_REQUESTS and LOAD's active_requests() agree. Two small items remain: the release ledger does not record the breaking trait change, and the hosted input contract is not checked.
New findings: 1 minor, 1 nit
Nits (1)
lib/kv-router/src/scheduling/selector/hosted.rs:98[nit] Hosted id ordering is a public contract but never checked:WorkerInputViewtells plugin authors that hosted rows are "selectable workers in ascending worker-ID order", andselectdocumentsidsas ascending without duplicates. Nothing checks this. A picker that keeps state by index, such as the round-robin cursor planned for #17713, would silently skew or repeat workers if a host passes an unsorted or duplicated slice, and no test would fail. Adebug_assert!costs nothing in release builds and catches a broken host in the routing-host tests.
The next AI round runs after new commits are pushed.
Keep both v1.7.0 deprecations.mdx entries. Port main's new dynamo-sticky-min-load picker to the &self WorkerPicker trait by holding its session bindings behind a lock. Signed-off-by: Karen Chung <karenc@nvidia.com>
ishandhanani
left a comment
There was a problem hiding this comment.
Round 2 AI review
Automated review (round 2 of up to 2 before a human review). Findings may be wrong; reply on a thread to dispute one.
Since round 1, the only change is a merge from main. It brings in sticky_min_load.rs, which this PR now adapts to &self by wrapping the existing picker in a parking_lot::Mutex. The wrapper is correct. The policy declares LOAD, so it never runs on the hosted path, and on the KV path the single scheduler calls it one request at a time, so the lock is never contended. The tests still call the inner StickyMinLoadPicker directly. One minor documentation gap from round 1 remains: the release ledger does not record the breaking change to the plugin trait signatures.
New findings: none
Earlier findings: 0 fixed, 0 no longer apply, 1 still open
- still open [DESIGN] Ledger records the OCCUPANCY rename but not the breaking trait change
AI review is done; a human reviewer takes it from here.
ishandhanani
left a comment
There was a problem hiding this comment.
lgtm - address rest of my comments
Co-authored-by: ishandhanani <82981111+ishandhanani@users.noreply.github.com> Signed-off-by: Karen Chung <karenc@nvidia.com>
…cleanup Bring in main through PR 2, now that #17712 has landed, along with PR 2's occupancy fix and release notes. Signed-off-by: Karen Chung <karenc@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
Router modes such as round-robin and least-loaded are "bespoke" code embedded into the core runtime / llm crates today. Worker-selection plugins allow for custom routing modes, but forces the SelectionCore/scheduler/indexer to be involved. So today, there is no way to port built-in
routing-mode's such as round-robin/random/least-loaded -- which today run on a specialized path with no SelectionCore involvement -- into the plugins without sacrificing these very simple algorithms' latencies.This PR makes plugins (1) performant/thread-safe to share across requests and (2) lets a plugin that needs no KV state run without the scheduler, with no lock and no per-request allocation.
The next PR in the stack uses this extended plugin capability to turn the built-in router modes into plugins.
Key Changes
Core logic: +423 / −88 lines (excluding tests, docs and comments). Most need review:
plugins/worker_selection.rs: plugin methods take&self, and the traits requireSend + Sync.selector/hosted.rs:HostedWorkerPicker::selectandHostedSelectionRequest.worker_selection/inputs.rs:WorkerInputViewbuilds rows lazily, and addsACTIVE_REQUESTS.New:
WorkerSelectionPolicy::into_hosted_picker()returns aHostedWorkerPickerfor a policy that has only a picker, declares no KV inputs and has no runtime config. Otherwise it returns the policy unchanged. The next PR in the stack calls it to run the router modes.ACTIVE_REQUESTSinput, replacingOCCUPANCY. Each worker's in-flight request count, which a picker reads withactive_requests(row)on either path. On the KV path it equalsLOAD'sactive_requests().OCCUPANCYstays as a deprecated alias until v1.9.OCCUPANCYwas a flag onlydynamo-llm's built-in selector used, and a plugin had no way to read the count. The rename from Occupancy to Active_requests is optional, but it feels like "Occupancy" (request count) sounds too similar to "Load" (token+block usage)..WorkerInputView::len()andis_empty(). A picker can choose by index without the host building rows.candidates()still works, building rows on first call.candidates(), which on the hosted path means building a row per worker on every request. Random and round-robin need only the count, and least-loaded and power-of-two need the count plusactive_requests(row).Changed:
keep,scoreandpicktake&selfinstead of&mut self, and the traits requireSend + Sync. One plugin instance can then serve every request with no lock, and the compiler checks its thread safety. A plugin that keeps state between calls uses atomics or a lock. The KV path is unchanged: one scheduler still calls each instance serially.self: a stack local in the scorer and a thread-local buffer in the picker. It allocates nothing new.ACTIVE_REQUESTSinput, and a deprecation entry forOCCUPANCY.Validation
ACTIVE_REQUESTSon the KV path matchesLOAD's active-request count, including for a worker with no load data.main(criterion; both built with the same settings, andmainrun twice to measure run-to-run noise):main;default_selection): 7.2 µs, 119.7 µs and 745.1 µs at 64, 1024 and 4096 workers, against 7.2–7.3 µs, 119.3–119.6 µs and 745.8–749.3 µs onmain;worker_selection,selection_core): within run-to-run noise. One shared-cache group looked slower in the first run; three alternating reruns againstmainput it at −2% to +4%.Issues and Ongoing Work
Pull Request Checklist
/pr-checklistskill.Summary by CodeRabbit
OCCUPANCYas deprecated in favor ofACTIVE_REQUESTS, with removal planned for version 1.9.