Skip to content

feat(router): expose worker inputs to request classifiers - #16616

Merged
ishandhanani merged 2 commits into
mainfrom
idhanani/adm-1-classifier-inputs
Oct 29, 2026
Merged

ishandhanani merged 2 commits into
mainfrom
idhanani/adm-1-classifier-inputs

Conversation

@ishandhanani

@ishandhanani ishandhanani commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Part of #14823. Replaces #16502, moved to an upstream branch so the admission capacity stack for #16543 can build on it (PR 1 of 5).

Summary

Request classifiers (the admission-control seam) now read the same per-worker WorkerInputs that worker-selection filters, scorers, and pickers use. Before this change a classifier only saw discovery data (block size and total KV blocks per rank), so admission plugins had to rebuild load from lifecycle events. This covers the #14823 items "let classifiers declare which data they need, like WorkerInputs, and skip optional work they do not need" and "prepare inputs in shared host code".

  • RequestClassifier::required_worker_inputs() declares the groups. The default is WorkerInputs::NONE, so existing classifiers, including ThunderAgent, build nothing and pay nothing.
  • classify receives the rows as workers: WorkerCandidates<'_>, a view restricted to the declared groups (empty for NONE). The host builds them for each call, lends them for the synchronous call only, and drops them before awaiting the classification future, so a deferred classification holds no per-rank memory. The borrow keeps the view from escaping into the 'static future.
  • Breaking for out-of-tree classifiers: RequestClassifier::classify gains the workers argument, matching how filters, scorers, and pickers receive their rows. A classifier that declares no inputs adds _workers: WorkerCandidates<'_> and ignores it. ThunderAgent and the in-tree tests and docs are updated.
  • The row builder moved out of MaterializedSelectionInput into worker_input_row, which selection and classification both call, so CACHE, LOAD, and PREFERRED_TAINT mean the same thing in both seams.
  • If LOAD is declared, the classify path projects worker load with the same project_worker_loads_into the scheduler actor uses, then frees that projection after building the rows, so a queued request does not keep one map entry per worker.
classify (t0): rows for the classifier's mask -> admission decision, may hold the request
actor    (t1): rows for the selector's mask   -> worker choice and reservation

Each seam reads data when it decides, so the two snapshots are separate on purpose. Classifier rows are advisory: they cover eligible ranks including overloaded workers, and selection can pick a different worker.

Rows are a snapshot taken for the classify call. A classifier that defers copies what it needs into its future and must not treat that copy as current load later. A live view, for classifiers that hold requests and decide at release time such as ThunderAgent, will follow in a separate PR. ThunderAgent keeps its event-based accounting for now.

The admission-control guide documents the new inputs, their snapshot timing, and an example.

Validation

  • New contract test classifier_reads_declared_worker_inputs: a classifier declaring CACHE | LOAD sees both eligible ranks with device overlap, is_available, and active requests. It fails as expected when the classify-time load projection is disabled. Existing classifier tests assert workers is empty for WorkerInputs::NONE.
  • A retry whose lifecycle already carries classification overrides skips the load projection and the row build: classify_request asks the runtime once (cached_overrides) and passes no inputs, since classify_with returns the cached overrides before the plugin runs. Test cached_overrides_follow_the_lifecycle.
  • cargo test -p dynamo-kv-router --features standalone-indexer --lib: 1169 passed. cargo test -p dynamo-custom-policy-builtin: all passed, including ThunderAgent. plugin_api_compat: passed. cargo clippy with -D warnings: clean on dynamo-kv-router and dynamo-custom-policy-builtin (all targets) and on the workspace. pre-commit: passed.
  • The Rust example added to custom-admission-control.md compiles against the public API. It counts only observed loads and requires at least one, per CodeRabbit's review of feat(router): expose worker inputs to request classifiers #16502.
  • Worker-selection hot path (measured before rebasing onto current main; the code moved, it did not change): in release builds collect_policy_candidates is the same size (2720 B) and its two inlined closures changed by +62 B and -100 B. An interleaved A/B of worker_selection benches at 10k workers on a shared, busy host was within noise (run-to-run spread 20-40%, overlapping ranges).

Summary by CodeRabbit

  • New Features

    • Request classifiers can now access snapshots of eligible workers’ availability, active-request load, and cache information.
    • Classifiers can use this data to make more informed routing decisions, such as selecting policies based on current worker capacity.
    • Worker-input snapshots remain consistent throughout each classification request.
  • Documentation

    • Added guidance and examples for declaring and reading worker-selection inputs.
    • Documented input groups, worker eligibility, snapshot timing, ordering, and callback usage.

Stack

  1. feat(router): expose worker inputs to request classifiers #16616 feat(router): expose worker inputs to request classifiers (this PR)
  2. refactor(router): judge queue capacity on the shared LOAD row #16617 refactor(router): judge queue capacity on the shared LOAD row
  3. feat(router): add a decode busy threshold to policy-class queues #16618 feat(router): add a decode busy threshold to policy-class queues
  4. feat(router): enforce busy-reject thresholds in the KV router queue #16619 feat(router): enforce busy-reject thresholds in the KV router queue
  5. refactor(router): unify classifier inputs; deprecate absolute busy thresholds #16620 refactor(router): rename classifier contexts; deprecate absolute busy thresholds

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

@ishandhanani
ishandhanani added this pull request to stack #16622 October 16, 2026 03:03
@devin-ai-integration

devin-ai-integration Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

✅ Dynamo PR CI passed — run 40457088283 (attempt 1) on f6747639a5

Gate checks: ✅ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Posted automatically by Devin for run 40457088283. Updated on every full-CI run of this PR.

@ishandhanani ishandhanani left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed with the whole stack (#16616 to #16620) checked out at the tip. Locally: cargo test -p dynamo-kv-router --features standalone-indexer --lib 1164 passed, clippy -D warnings clean on kv-router, mocker, and llm.

The seam is sound: one row builder for both consumers, NONE builds nothing, snapshot timing is documented. Two inline notes, on the classify path's per-request allocation and on the shared builder's inlining.

Comment thread lib/kv-router/src/scheduling/local.rs Resolved
@ishandhanani
ishandhanani force-pushed the idhanani/adm-1-classifier-inputs branch from 3122234 to c084701 Compare October 19, 2026 05:46

@jthomson04 jthomson04 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two notes on the classifier rows: memory held per pending classification, and work done under the worker-config read guard.

Comment thread lib/kv-router/src/scheduling/queue.rs Resolved
@ishandhanani
ishandhanani marked this pull request as ready for review October 20, 2026 04:04
@ishandhanani
ishandhanani requested review from a team as code owners October 20, 2026 04:04
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 68dfe27b-c8a6-4f60-9496-9bcdae05ae38
📥 Commits

Reviewing files that changed from the base of the PR and between 92a28f8 and c084701.

📒 Files selected for processing: 8
  • docs/fern/pages/developer-guide/knowledge-base/modular-components/router/custom-admission-control.md
  • lib/kv-router/src/plugins/request_classifier.rs
  • lib/kv-router/src/plugins/worker_selection.rs
  • lib/kv-router/src/plugins/worker_selection/inputs.rs
  • lib/kv-router/src/scheduling/local.rs
  • lib/kv-router/src/scheduling/queue.rs
  • lib/kv-router/src/scheduling/request_classifier.rs
  • lib/kv-router/src/scheduling/selector/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

Classifiers can now declare worker-selection input groups and read a callback-scoped snapshot of eligible workers’ cache and load data. The scheduler captures declarations, builds requested rows, and supplies them before classification. Documentation and tests cover load-aware classification and snapshot behavior.

Changes

Classifier Worker-Input Snapshots

Layer / File(s) Summary
Declare and read worker inputs
lib/kv-router/src/plugins/request_classifier.rs, lib/kv-router/src/plugins/worker_selection.rs, lib/kv-router/src/plugins/worker_selection/inputs.rs
RequestClassifier can declare required worker inputs. ClassifyRequest can store worker-input rows and expose them through a scoped callback.
Capture declarations and build worker rows
lib/kv-router/src/scheduling/request_classifier.rs, lib/kv-router/src/scheduling/selector/mod.rs
The runtime captures each classifier’s input declaration. Selector helpers construct requested cache and load data for eligible workers.
Attach worker rows to classification requests
lib/kv-router/src/scheduling/queue.rs
SchedulerQueue accepts a WorkerInputs mask and attaches rows when the mask is non-empty. Existing tests and benchmarks pass WorkerInputs::NONE.
Prepare and read snapshots during classification
lib/kv-router/src/scheduling/local.rs, docs/fern/pages/developer-guide/knowledge-base/modular-components/router/custom-admission-control.md
LocalScheduler adds requested load projections before classification and clears them afterward. Tests and documentation describe input declarations and snapshot behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🔵 Low · up to c0847

Load-aware classification may briefly delay worker updates and use more memory for deferred requests. These bounded risks warrant owner awareness but do not currently block merging.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 61.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 7 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The PR description is substantive and covers the change well: it summarizes the new RequestClassifier::required_worker_inputs() declaration, the read_worker_inputs_snapshot view, the breaking change t… 1. Add the template headings: 'Overview', 'Details', and 'Where should the reviewer start?'. 2. Move the summary content into 'Overview' and 'Details'. 3. Name the files a reviewer should read first, for example request_classifier.rs, sched…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: exposing worker inputs to request classifiers.
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.
Full details: Docstring Coverage

Explanation

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

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The PR description is substantive and covers the change well: it summarizes the new RequestClassifier::required_worker_inputs() declaration, the read_worker_inputs_snapshot view, the breaking change to classify (new workers argument), the shared worker_input_row builder, the LOAD projection and its release, snapshot timing, and a validation section with tests and commands. It has a Stack list and links to the parent issue (#14823). It does not follow the repository template: the required 'Overview', 'Details', and 'Where should the reviewer start?' headings are missing, and the 'Related Issues' section does not use the required 'Closes #XXXX' or 'Relates to #XXXX' format. The author writes 'Part of #14823', which references the issue but does not close or relate it under the template's keywords.

Resolution

  1. Add the template headings: 'Overview', 'Details', and 'Where should the reviewer start?'. 2. Move the summary content into 'Overview' and 'Details'. 3. Name the files a reviewer should read first, for example request_classifier.rs, scheduling/local.rs, and selector/mod.rs. 4. In 'Related Issues', add 'Relates to #14823' (or 'Closes #14823' if this PR completes that issue). Keep the stack list. 5. Fix the test count discrepancy: the description says 1169 tests passed, but the objectives state 1155 passed. Confirm the correct number and update the text.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@ishandhanani
ishandhanani force-pushed the idhanani/adm-1-classifier-inputs branch from c084701 to 7035301 Compare October 20, 2026 07:08
@ishandhanani
ishandhanani requested a review from a team as a code owner October 20, 2026 07:08
@github-actions github-actions Bot added the router Relates to routing, KV-aware routing, etc. label Oct 20, 2026
@ishandhanani
ishandhanani force-pushed the idhanani/adm-1-classifier-inputs branch 3 times, most recently from af73e78 to 91fc895 Compare October 22, 2026 12:51

@jthomson04 jthomson04 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One perf note on retries in the classify path.

Comment thread lib/kv-router/src/scheduling/local.rs Resolved
@ishandhanani
ishandhanani force-pushed the idhanani/adm-1-classifier-inputs branch from 91fc895 to 3918a95 Compare October 23, 2026 02:13

@jthomson04 jthomson04 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Retry-path fix verified on 3918a95.

@ishandhanani
ishandhanani force-pushed the idhanani/adm-1-classifier-inputs branch 2 times, most recently from 8fa11cf to 5bfcfef Compare October 24, 2026 05:37
Admission plugins can now declare `WorkerInputs` with
`RequestClassifier::required_worker_inputs` and receive per-worker rows as
the `workers` argument of `classify`. Rows come from the row builder worker
selection uses, so CACHE, LOAD, and PREFERRED_TAINT mean the same thing in
both seams. The default `WorkerInputs::NONE` builds nothing.

The rows are a snapshot lent for the synchronous `classify` call only; the
host drops them before awaiting the classification future, so a deferred
classification does not hold memory per worker rank. Worker selection still
takes its own snapshot when it picks a worker.

A retry whose lifecycle already carries classification overrides
returns from `classify_with` before the plugin runs, so
`classify_request` asks the runtime first and skips the load projection
and the row build for it.

Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
@ishandhanani
ishandhanani force-pushed the idhanani/adm-1-classifier-inputs branch from 5bfcfef to 48b6b96 Compare October 25, 2026 05:47
Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>

# Conflicts:
#	lib/kv-router/src/scheduling/selector/mod.rs
@ishandhanani
ishandhanani merged commit 09dc99f into main Oct 29, 2026
124 checks passed
@ishandhanani
ishandhanani deleted the idhanani/adm-1-classifier-inputs branch October 29, 2026 05:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation feat router Relates to routing, KV-aware routing, etc. size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants