Repository navigation
fix(router): reject non-finite router scoring weights at config validation - #15529
Conversation
…ation KvRouterConfig::validate() checked prefill_load_scale, overlap_score_credit_decay and router_temperature with validate_min (value >= min), which accepts +inf. The builtin default policy then misbehaves per request instead of the router failing at startup: - prefill_load_scale=inf makes every candidate cost non-finite, so every selection fails with NonFiniteCost. - overlap_score_credit_decay=inf gives the least-loaded worker a decay factor of 1 / (1 + inf * 0) = NaN; the scorer's .max(0.0) turns that into a zero prefill cost and silently changes the routing decision. - router_temperature=inf degenerates sampling to uniform random. The builtin plugin already rejects these values when they arrive as policy parameters, and this file already requires finiteness for overlap_score_credit and, via validate_range(0.0, f64::MAX), for decode_active_request_weight. Add validate_finite_non_negative and use it for these weights in KvRouterConfig::validate() and RouterConfigOverride::validate(). Use it for decode_active_request_weight as well: the accepted values are unchanged, but the error no longer prints f64::MAX in full. Signed-off-by: Akira Lonske <akiralonske@gmail.com>
|
👋 Hi aylonsk! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (2)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 (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughRouter configuration validation now rejects negative and non-finite values for selected scoring settings. Per-request overrides also reject negative and non-finite prefill-load scales and router temperatures. Tests cover invalid values and acceptance of ChangesRouter setting validation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The validation changes match the intended behavior, with coverage for invalid values and maximum finite values. No actionable merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
jthomson04
left a comment
There was a problem hiding this comment.
Reviewed the source at 94ad621. No actionable findings. The validation matches the builtin policy rules and preserves accepted finite values. Tests and CI were not checked in this review.
|
Thank you for your review @jthomson04. If all looks good, I would appreciate full CI checks on this PR. |
|
/ok to test 94ad621 |
❌ Dynamo PR CI failed — run 37345566557 (attempt 1) on
|
| Framework | 1-GPU arm64 |
|---|---|
| SGLang | ❌ 1 |
| Other | Jobs |
|---|---|
| frontend | ❌ 1 |
Failure details
2 jobs failed, both on infrastructure errors: the frontend multi-arch CPU image build hit an HTTP 502 cloning libvpx from chromium.googlesource.com, and the SGLang arm64 CPU-test job lost its runner container (Executing the custom container implementation failed) after every test it ran had passed. Rerunning the failed jobs is likely enough.
❌ frontend / Build multi-arch cpu: libvpx git clone failed with HTTP 502
Job: frontend / Build multi-arch cpu · Failed step: Build and Push Image · Logs: gh run view --job 111883341912 -R ai-dynamo/dynamo --log-failed
#130 [linux/amd64 wheel_builder_base 17/19] RUN ... git clone --depth 1 --branch v1.14.1 https://chromium.googlesource.com/webm/libvpx.git ...
#130 24.96 Cloning into 'libvpx'...
#130 51.33 error: RPC failed; HTTP 502 curl 22 The requested URL returned error: 502
#130 51.33 fatal: expected flush after ref listing
#130 ERROR: process "/bin/sh -c ... git clone --depth 1 --branch ${LIBVPX_REF} https://chromium.googlesource.com/webm/libvpx.git ..." did not complete successfully: exit code: 128
ERROR: failed to build: failed to solve: process "/bin/sh -c ..." did not complete successfully: exit code: 128
##[error]Process completed with exit code 1.
The linux/amd64 wheel_builder_base stage failed while cloning libvpx v1.14.1 because chromium.googlesource.com returned HTTP 502. This is an upstream/network infrastructure error, not a compile failure. Rerunning the failed jobs is likely enough.
❌ sglang-runtime / Test cuda13.0, arm64: runner container hook failed during CPU-only tests
Job: sglang-runtime / Test cuda13.0, arm64 · Failed step: Run CPU-only tests (parallelized) · Logs: gh run view --job 111896807038 -R ai-dynamo/dynamo --log-failed
[gw3] [100%] RETRY PASSED (Auto Test Retries) tests/frontend/test_sglang_generate_mocker.py::test_native_generate_replays_incremental_ids_and_keeps_openai_canonical
[gw3] [100%] RETRY FAILED (Auto Test Retries) tests/frontend/test_sglang_generate_mocker.py::test_native_generate_replays_incremental_ids_and_keeps_openai_canonical
[gw3] [100%] RETRY PASSED (Auto Test Retries) tests/frontend/test_sglang_generate_mocker.py::test_native_generate_replays_incremental_ids_and_keeps_openai_canonical
[gw3] [100%] PASSED tests/frontend/test_sglang_generate_mocker.py::test_native_generate_replays_incremental_ids_and_keeps_openai_canonical
##[error]Executing the custom container implementation failed. Please contact your self hosted runner administrator.
##[end-action id=cpu-only-tests.__run;outcome=failure;conclusion=failure;duration_ms=600059]
...
##[warning]No files were found with the provided path: test-results/pytest_test_report_*.xml. No artifacts will be uploaded.
❌ pre_merge_cpu: step outcome 'failure'
The pytest run reached 100% with no test marked failed. test_native_generate_replays_incremental_ids_and_keeps_openai_canonical failed one auto-retry but ended PASSED. The self-hosted k8s runner's container hook then failed at about 600 s, before pytest printed its summary or wrote the JUnit XML, and every later step failed with the same hook error. This is a runner/container infrastructure failure. Rerunning the failed jobs is likely enough.
For agents
{"pr": 15529, "run_id": 37345566557, "run_attempt": 1, "head_sha": "94ad621383d73f4fc8bb2cf5e2760cb8c54cb4b8", "failures": [{"job": "frontend / Build multi-arch cpu", "job_id": 111883341912, "failed_step": "Build and Push Image", "signature": "git clone libvpx from chromium.googlesource.com: RPC failed; HTTP 502 (infra)", "tests": [], "log_cmd": "gh run view --job 111883341912 -R ai-dynamo/dynamo --log-failed"}, {"job": "sglang-runtime / Test cuda13.0, arm64", "job_id": 111896807038, "failed_step": "Run CPU-only tests (parallelized)", "signature": "Executing the custom container implementation failed (self-hosted k8s runner container hook, infra)", "tests": [], "log_cmd": "gh run view --job 111896807038 -R ai-dynamo/dynamo --log-failed"}]}Posted automatically by Devin for run 37345566557. Updated on every full-CI run of this PR.
Overview:
KvRouterConfig::validate()checksprefill_load_scale,overlap_score_credit_decayandrouter_temperaturewithvalidate_min(value >= min), which accepts+inf.RouterConfigOverride::validate()does the same forprefill_load_scaleandrouter_temperature. A non-finite weight passes startup and then breaks the builtin default policy on every request. This PR rejects non-finite values for these weights at validation, the same way the builtin plugin already does for its policy parameters.Summary
validate_finite_non_negativetolib/kv-router/src/scheduling/config.rs. Its error,"{field} must be finite and non-negative", is the one the builtin plugin already uses for policy parameters.overlap_score_credit_decay,prefill_load_scaleandrouter_temperatureinKvRouterConfig::validate(), and forprefill_load_scaleandrouter_temperatureinRouterConfigOverride::validate().decode_active_request_weighttoo. That field already rejected non-finite values throughvalidate_range(0.0, f64::MAX), and it accepts the same values as before. Its error now readsdecode_active_request_weight must be finite and non-negativeinstead of printingf64::MAXas a 309-digit number.f64::MAX.Details:
Why these fields. The builtin default policy rejects exactly these values when they come from policy parameters (
lib/router-plugins/builtin/src/default/parameters.rs, "must be finite and non-negative"). ButParameters::resolvefalls back to theKvRouterConfigvalue when a parameter is unset, so values from env, CLI orKvRouterConfigreach the scorer unchecked. The same config file already requires finiteness foroverlap_score_credit, andpolicy_config.rsrequires it forprefill_busy_threshold_frac.Observed on
mainat 2de120f. Builtindefault_policy, 400 selections per row,router_temperature=0unless the row varies it:validate()prefill_load_scale=1Ok(())prefill_load_scale=infOk(())Err(NonFiniteCost { scorer_index: 0, row: 0 })× 400prefill_load_scale=inf,track_prefill_tokens=falseOk(())Err(NonFiniteCost { scorer_index: 0, row: 0 })× 400overlap_score_credit_decay=1or1e300Ok(())overlap_score_credit_decay=infOk(())router_temperature=1e300orinfOk(())What goes wrong for each field:
prefill_load_scale=inf: the cost becomesinf * prefill, orinf * 0 = NaN, so every candidate cost is non-finite.overlap_score_credit_decay=inf: the least-loaded worker's decay factor is1 / (1 + inf * 0) = NaN, and(prefill - NaN).max(0.0)returns0.0. That worker's prefill cost silently drops to zero, and the decision differs from the finite limit (1e300). In the decay rows, worker 0 rank 0 has the least prefill load but a decode backlog, and worker 1 rank 0 has 10 more prefill blocks and no backlog.router_temperature=inf: this is the uniform-sampling limit, so it doesn't fail. It's included for consistency with the plugin's parameter validation.How the values get in.
DYN_ROUTER_PREFILL_LOAD_SCALE,DYN_ROUTER_KV_OVERLAP_SCORE_CREDIT_DECAYandDYN_ROUTER_TEMPERATUREset toinf,Infinityor1e999all parse toinfand passvalidate().NaNwas already rejected. The Python--router-prefill-load-scale,--router-kv-overlap-score-credit-decayand--router-temperatureflags parse withfloat. The resultingKvRouterConfig(**config.kv_router_kwargs())binding validates on construction (lib/bindings/python/rust/llm/entrypoint.rs), andKvRoutercallsvalidate()afterapply_policy_config(). With this change, all of these paths fail at startup, for example withprefill_load_scale must be finite and non-negative. #15202 deprecated these env vars and flags in favor of the default policy parameters (removal planned for v1.7), and those parameters already reject non-finite values. Until then,Parameters::resolvestill falls back to theseKvRouterConfigvalues whenever a policy parameter is unset.Behavior change. A router configured with
router_temperature=infused to route uniformly at random; it now fails validation. A large finite temperature gives the same distribution. No in-repo code sets any of these fields to a non-finite value.Validation
cargo test -p dynamo-kv-router --lib: 1026 passed, 0 failed, 2 ignored.cargo test -p dynamo-custom-policy-builtin: all suites pass (48, 5, 1, 2, 13 and 4 tests).cargo clippy -p dynamo-kv-router -p dynamo-custom-policy-builtin --all-targets -- -D warnings: clean.cargo fmt --all -- --check: clean.pre-commit run --files lib/kv-router/src/scheduling/config.rs: passed.scoring_weights_must_be_finite_and_non_negativechecks the exact error for each field at-0.1,NaNandinf, for both structs. It also checks thatf64::MAXis still accepted.dynamo_env_config_ignores_unparseable_values_and_validates_rangesnow covers the three env vars alongside the two it already checked.infrow failsvalidate()with the field named in the message.Where should the reviewer start?
lib/kv-router/src/scheduling/config.rs:validate_finite_non_negativeand its call sites inRouterConfigOverride::validate()andKvRouterConfig::validate().Related Issues
Summary by CodeRabbit