Repository navigation
fix(auto-routing): survive container rollouts and make scheduler limits admin-configurable - #7614
Conversation
Raise the default per-model benchmark concurrency from 4 to 12 and let admins override slots, model, provider and unknown-provider concurrency at runtime through a global scheduler_settings D1 row, exposed as GET/PUT /admin/scheduler-config and a Scheduler card on the admin Benchmarks tab.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit classifies container outcomes by error type so only a genuinely never-started case is reported as Overview
Files Reviewed (2 files in incremental diff)
Notes (non-blocking, no action required):
Previous Review Summaries (3 snapshots, latest commit fac1ed2)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fac1ed2)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental changes since the last review are formatting-only (oxfmt reflow) plus cast removals in Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files in incremental diff, plus prior review scope)
Fix these issues in Kilo Cloud Previous review (commit 150163d)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryIncremental review of Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (11 files)
Fix these issues in Kilo Cloud Previous review (commit 50b3e7e)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the admin-tunable benchmark scheduler limits change (new single-row Files Reviewed (15 files)
Notes (non-blocking, no action required):
Reviewed by deepseek-v4.1-flash · Input: 49.3K · Output: 10.5K · Cached: 310.3K Review guidance: REVIEW.md from base branch |
A worker deploy rolls every container over, which tears down the caller's connection while a benchmark case is in flight. The container drained the CLI child and wrote the output to a socket nobody was reading, so the retry found the paid-execution ledger entry without pending output and killed the lane. The container now refuses cases it has not started when SIGTERM arrives, so a rollout cannot pay for work it interrupts, and posts the outcome of a case it did drain back to the worker's /container/case-result route. That route is authenticated by the paid-execution claim token, which a container can hold and the backend secret it cannot, and only accepts outcomes for a claim token that opened the execution: a completed run becomes recoverable pending output, and a case that never started releases its ledger entry. A connection lost in flight keeps quarantining the slot with the chunk's lease instead of recording a lane failure, so lease recovery re-dispatches the chunk and reuses the delivered output. Only a case that reaches the ledger with no recoverable output by recovery time remains ambiguous and still fails its lane rather than repeating a paid call.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
iscekic
changed the title
A container that lost its caller posted every case failure as not_started, which releases the paid-execution ledger row. A failure after the CLI spawned can have made a paid call, so that row must survive for ambiguity quarantine.
|
bot: Admin Scheduler card verified end to end in a browser against a local benchmark worker and local D1 (no production contact): default state, save an override, reload persists, clear back to default, and an API-rejected value shows an error toast with the card intact.
|
Summary
Two benchmark robustness problems, fixed together.
1. Every worker deploy killed ~15 measurement lanes
Every deploy from the pipeline (including unrelated
packages/**changes, which redeploy every worker) rolls the benchmark containers over. Axiom (cloudflare-logpush,ScriptNameauto-routing-benchmark) shows the sequence:Container error: Runtime signalled the container to exit due to a new version rollout, thenNetwork connection lost/Container port connection closed unexpectedly/The container just exitedon the in-flight/runproxy, thenbenchmark_scheduler_wait reason=ambiguous_execution.The container's SIGTERM drain finished the in-flight CLI case and wrote the response to a connection that no longer existed, so the output was lost even though the case had been paid for. On the retry, the paid-execution ledger still had the row, no pending output existed, and the scheduler killed the whole
(model, variant, rep)lane.503), so a rollout never pays for work it interrupts, and drains the case it already accepted./container/case-resultroute, which stores the outcome as recoverable pending output. That route is authenticated by the paid-execution claim token (which a container can hold, and the backend secret cannot) and only accepts an outcome for a claim token that opened an execution. Anot_startedoutcome releases the ledger row, so the case is retried without a paid call.2. Scheduler limits are admin-configurable
Benchmark evals are bottlenecked by the per-model chunk cap: one model lane can only run a few chunks at once, so a single large lane drains slowly while the shared container pool sits idle.
modelConcurrencyfrom 4 to 12. The other scheduler defaults are unchanged.slots,modelConcurrency,providerConcurrency, andunknownProviderConcurrencyat runtime, without a deploy.BenchmarkConfigor/admin/config.scheduler_settings, generated withpnpm db:generate. The migration only adds a table.dispatchSchedulerreads the row once per pass by primary key, resolves the effective config, and passes it toclaimScheduledChunksasconfig. Nothing reads the row per chunk.BENCHMARK_SCHEDULER_CONFIG, then the default.modelLimits,providerLimits, andsharesstill come only from the env config.GET /admin/scheduler-configreturns each limit's effective value, its source (admin,env, ordefault), the override, and the fallback used without one.PUT /admin/scheduler-configreplaces the whole override set. A missing ornulllimit clears its override. The request is rejected with a 400 for any of these:BENCHMARK_CONTAINER_BUDGET/adminbearer auth and thex-updated-byconvention./admin/api/auto-routing/benchmark-scheduler-configand the shared admin client, and the request/response contracts live in@kilocode/auto-routing-contracts.Verification
services/auto-routing-benchmarkon a disposable VM:pnpm typecheck,pnpm lint(oxlint, 0 errors),pnpm test(677 tests),pnpm smoke:schedulerwrangler devwith local D1 and every migration applied, thenGET /admin/scheduler-config(401 unauthenticated;modelConcurrency12 with sourcedefault),PUT {"modelConcurrency":8}(source flips toadmin, and the next GET returns it), and invalid PUTs (modelConcurrency: 0, unknown key) rejected with 400Rollout tests cover:
not_started503) once shutdown begins, so no paid call starts after SIGTERMnot_started, and rejects an outcome whose claim token does not match an open executionThe scheduler-configuration commit, as reported on this PR before it was combined:
packages/auto-routing-contracts:pnpm typecheck,pnpm lint,pnpm testpackages/web-shared:pnpm typecheckapps/web:pnpm typecheck, oxlint on the changed admin paths, jest for the auto-routing admin page and routes and for the benchmark admin clientResidual risks
Visual Changes