Repository navigation
feat(router): unify native plugin setup and register classifiers - #14149
Conversation
625a1c9 to
9ae3a91
Compare
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Reviewed at 8ac1507.
Stack. This is the 4th PR in a stack — main -> #14176 -> #14177 -> #14123 -> this PR. gh api repos/ai-dynamo/dynamo/compare/b9cb055...8ac1507 reports merge-base b9cb055 (the head of #14123), 3 commits ahead, 0 behind, 22 files. Everything below is scoped to those 3 commits. #14570 is a separate series off main by the same author, not part of this stack, and I did not restate anything belonging to it or to #14123/#14177.
Intent, from the diff and the description: register worker-selection and request-classifier plugins through one catalog, resolve them once into a RouterPlugins bundle, and attach that bundle through one router-construction path shared by HTTP discovery and the Python KvRouter. "Unify" plus "register" means the risk is a plugin silently not registering, registering twice, or the default path quietly changing, so that is what I measured.
Registered-set diff, base vs head — by execution
I enumerated the registry at runtime rather than reading call sites, by forcing the UnknownType error whose available field lists every registered type, sorted:
base b9cb055 |
head 8ac1507 |
|
|---|---|---|
| worker-selection types | dynamo-two-tier-cost-fn |
dynamo-two-tier-cost-fn |
| request-classifier types | (no such registry) | <none> |
Nothing stopped being registered, nothing is registered twice, and the shipped catalogs register zero classifier types — so no default image can pick one up. (dynamo-worker-selection-policy-catalog::register is Ok(()) with an unused parameter on both trees, so the whole set comes from dynamo-custom-policy-builtin.)
Three no-plugin configurations resolve to no factory on both trees: no router_policy_config at all, a profile-only examples/router/policy-class-queues.yaml, and an explicit worker_selection: {aggregated: default}.
Registration failure is loud, not degrading. Running the real router_plugins() helper in the bindings crate:
default (nothing configured) -> is_empty=true ws=false rc=false
request_classifier: {type: acme} -> Err: "request_classifier is configured, but no router
plugin catalog is installed; rebuild with --features custom-policy"
worker_selection instance "a" -> Err: "worker-selection instance \"a\" is configured, but
this Dynamo build has no linked router plugin catalog; rebuild with --features custom-policy"
Double registration is rejected (Duplicate), default is reserved on both registries, and both available_*_types() sort before joining while resolve_selections walks roles in fixed order — so no HashMap-iteration nondeterminism leaks into messages or into selection.
Default-path differential — byte-identical
Canonical offline replay, testdata/mooncake_trace_1000.jsonl, --num-workers 8, KV router mode, --features replay-bench, run on base and head from separate worktrees:
canon-base-1000.jsonl 1394604 bytes
canon-head-1000.jsonl 1394604 bytes
cmp -> byte-identical
Each of the 1,000 per_request records carries decode_worker_idx plus a routing_history with logical_worker_id, scheduler_id, queue_wait_ms and selected_overlap_blocks, so this compares the chosen worker for every decision, not aggregates.
That covers the scheduling/selection core, but replay builds its selector through replay_selector, not through the new RouterPluginBuilder, so I checked the construction path separately:
- Frontend (
ModelWatcher) — head'sRouterPluginBuilder::<DefaultWorkerSelector>::default()closure body is character-for-character the closureModelWatcher::newpassed at base, andkv_chooser_for_with_plugins_and_clientfeeds it the same(config, effective_worker_type(card), RoutingPartitionRef::new(&card.display_name, DEFAULT_ROUTING_GROUP)). Unchanged. - Standalone Python
KvRouter— this one genuinely changed expression, from themetric_worker_typestring toselector_worker_role.default_selector_label(). I ran the realinfer_metric_worker_typeover a 1,458-case grid inside the bindings crate: its codomain is exactly{"decode","prefill"}, and head'sWorkerTypemapping reproduces the identical string in every case. The companionOption<KvRouterConfig>->Some(unwrap_or_default())change is unreachable — both callers ofcreate_kv_router_from_endpointpassSome(..).
Test coverage is load-bearing
discovery::watcher::tests::plugin_bundle_is_installed_per_discovered_model passes locally (cargo test -p dynamo-llm --no-default-features --lib). Replacing plugins.install(&chooser)? in kv_chooser_for_with_plugins_and_client with a no-op makes it fail (timeout waiting for the classify event), so it pins the wiring and not just the types.
Also checked
cargo clippy --all-targets -- -D warningsonlib/bindings/pythonclean locally for both--features custom-policy,select-serviceand--no-default-features, matching the greenrust-clippy (lib/bindings/python)job which runs exactly those two.- Every supported entry point rejects a configured classifier it cannot honour rather than ignoring it: stock build (bail), non-KV router mode (bail), non-HTTP input (bail), static engine (bail), standalone selection service (bail), offline replay (bail). Prefill-hop and low-level
best_workerexclusions are stated in the new docs. request_classifierconfig is operator-supplied startup YAML,deny_unknown_fields, identifier-validated, parameters must be a mapping. No request-derived input reaches the registry, so no new caller-influenced echo.
One P3 inline, not blocking.
Not verified: no live multi-node frontend was available, so the end-to-end differential is the offline-replay one above plus the construction-path equivalence — not a live A/B of two running frontends. Behaviour of the Encode-typed path was inconclusive: an Encode card in the new watcher test resolves to Aggregated before reaching the chooser, so I could not establish whether an Encode router ever receives a classifier, and I am not reporting it either way.
c63a675 to
af6ceaf
Compare
af6ceaf to
c0de46b
Compare
8dd0051 to
ca756b4
Compare
ca756b4 to
05747ac
Compare
05747ac to
baf3a8c
Compare
baf3a8c to
e3bc02d
Compare
Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
e3bc02d to
1391c89
Compare
|
/ok-to-test 1391c89 |
1 similar comment
|
/ok-to-test 1391c89 |
Summary
Register worker-selection and request-classifier plugins through one catalog, resolve them into one
RouterPluginsbundle, and attach them through shared router construction used by HTTP discovery and PythonKvRouter.generate.request_classifier: {type, parameters}selects a provider that validates configuration at startup and creates fresh classifier state per router. Each classifier factory receives theRequestClassifierContextrestored in #14123. The shared builder supplies that router's cached capacity view; live request progress and advisory worker targets come from the same base. Rust embedders pass the resolved bundle withHttpFrontend::plugins(...). The existing worker-only catalog API remains compatible. Construction uses the shared SelectionCore introduced by #14570; no generic selector builder is needed.flowchart LR Catalog["catalog::register"] --> Registry[RouterPluginRegistry] YAML[Router policy YAML] --> Registry Registry -->|resolve_plugins| Bundle[RouterPlugins] Bundle --> HTTP[HTTP discovery] Bundle --> Python[Python KvRouter] HTTP --> Build[Shared router construction] Python --> Build Build --> Selector[Worker selector] Build --> Classifier[Request classifier]One installed registry serves the frontend and Python bindings. The shared builder passes worker selection into SelectionCore preparation (one factory call per partition, including the required-input check), then installs the classifier on the resulting router. Native HTTP and gRPC reject classifier YAML when no classifier is installed.
New plugin traits add a typed registry entry, bundle factory, and construction hook; frontend and binding entry points continue to carry one bundle. Execution and lifecycle callbacks remain explicit on each trait, with no new per-request dispatch.
Plugin-facing contracts, signal accessors, configuration, and providers live under
plugins::worker_selectionandplugins::request_classifier. Scheduling owns execution, eligibility, reservations, and lifecycle synchronization. The signals restored in #14123 retain their semantics when moved into the canonical plugin module.RequestProgressand its existing scheduling imports remain the same type. Classifier factories now accept their per-router capacity context. Existing imports,WorkerSelectionPolicyRegistry, worker-onlyregister, and the frontend worker-selection factory setter remain compatible, with explicitTODO(v1.7)removal comments. The compatibility exports preserve type identity; old plugins need no adapters.Based on #14123 for the classifier interface; implements the catalog wiring for DEP #13891. Replay and standalone selection reject classifiers explicitly. Prefill classification and low-level/EPP lifecycle enrollment remain outside this change, with lifecycle work tracked in #14823.
Validation
custom-policy,select-serviceand with no default features.Earlier description and automated notes (superseded by the current summary above)
Summary
Register worker-selection and request-classifier plugins through one catalog, resolve them into one
RouterPluginsbundle, and attach them through shared router construction used by HTTP discovery and PythonKvRouter.generate.request_classifier: {type, parameters}selects a provider that validates configuration at startup and creates fresh classifier state per router. Rust embedders pass the resolved bundle withHttpFrontend::plugins(...). The existing worker-only catalog API and concrete default selector remain compatible.flowchart LR Catalog["catalog::register"] --> Registry[RouterPluginRegistry] YAML[Router policy YAML] --> Registry Registry -->|resolve_plugins| Bundle[RouterPlugins] Bundle --> HTTP[HTTP discovery] Bundle --> Python[Python KvRouter] HTTP --> Build[Shared router construction] Python --> Build Build --> Selector[Worker selector] Build --> Classifier[Request classifier]New plugin traits add a typed registry entry, bundle factory, and construction hook; frontend and binding entry points continue to carry one bundle. Execution and lifecycle callbacks remain explicit on each trait, with no new per-request dispatch.
Plugin-facing contracts, signal accessors, configuration, and providers live under
plugins::worker_selectionandplugins::request_classifier. Scheduling owns execution, eligibility, reservations, and lifecycle synchronization. Signal APIs and semantics are unchanged. Existing imports,WorkerSelectionPolicyRegistry, worker-onlyregister, and the frontend worker-selection factory setter remain compatible, with explicitTODO(v1.7)removal comments. The compatibility exports preserve type identity; old plugins need no adapters.Based on #14123 for the classifier interface; implements the catalog wiring for DEP #13891. Replay and standalone selection reject classifiers explicitly. Prefill classification and low-level/EPP lifecycle enrollment remain outside this change, with lifecycle work tracked in #14823.
Validation
custom-policy,select-serviceand with no default features.Automated notes from the previous revision (awaiting refresh)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation