feat(warmup): proactive Claude warmup scheduler (#8848) - #9449
Merged
diegosouzapw merged 11 commits intoAug 7, 2026
Merged
Conversation
Cron-driven warmup for opted-in Anthropic OAuth connections, so a rate-limit window is triggered ahead of real traffic instead of by it. - warmupScheduler.ts: PT-cron tick, opt-in gating, circuit breaker skip, 401 refresh+retry, 403/429 classification, proxy via runWithProxyContext, message rotation, Undici body cleanup on all paths - warmupScheduler/*: core types, Redis + SQLite circuit breaker stores, exponential backoff (5m base, 240m cap), factory with Redis to SQLite fallback - connectionRuntimeState: column-level atomic UPSERT for warmup circuit state - migration 135: connection_runtime_state table (134 is taken by 134_proxy_logs_egress_ip.sql on this branch) - localDb: re-export the new db module - settings: claudeWarmup opt-in schema + default (empty = off) - initCloudSync: mount startWarmupScheduler after startBudgetResetJob - .env.example + docs/reference/ENVIRONMENT.md: OMNIROUTE_WARMUP_ENABLED, _CRON, _CONCURRENCY, _MODEL Closes diegosouzapw#8848 Signed-off-by: Minxi Hou <houminxi@gmail.com>
- 30 tests across 6 files: orchestrator integration (opt-in gating, Bearer+beta auth, message rotation, 401 retry, 403 forbidden persist, 429 Retry-After parse, api_key skip, terminal skip), Redis + SQLite circuit breaker stores, factory Redis-to-SQLite fallback, backoff curve, column-level atomic UPSERT for connection_runtime_state. - __resetWarmupState() test hook: resets the globalThis singleton so lastFireMinute/minuteKey latch from a prior test does not suppress the tick in the next test (test-ordering isolation). - stopWarmupScheduler() resets lastFireMinute so a subsequent start() can fire immediately (correct restart semantics). - redisCircuitBreakerStore: complete the SQLite dual-write for forbidden so the flag survives Redis eviction (best-effort, non-fatal). Signed-off-by: Minxi Hou <houminxi@gmail.com>
redisCircuitBreakerStore recorded success in Redis but left the SQLite
backup's last_warmup_result at 'forbidden'. A Redis key eviction then made
the SQLite fallback read that stale flag and block every future warmup for
that connection.
clearWarmupCircuit is not enough here: it resets streak, until and
lastFailAt but never touches last_warmup_result. Use
upsertWarmupState({ lastResult: 'success' }) instead, which is what the
SQLite store's own success path already does.
Also drop a dead failForbidden mock from the test. _setFailForbidden could
not mutate it, since JS copies the value, so it asserted nothing. The
dual-write path is best-effort and non-fatal, and the SQLite store's own
tests cover it.
The new test asserts the SQLite backup's final state rather than merely
that the call happened, verified by injecting the regression and watching
it fail before the fix.
Signed-off-by: Minxi Hou <houminxi@gmail.com>
5 tasks
…lure getCircuitBreakerStore caches the store it builds and returns it on every later call. The Redis store does not catch its own errors, so once Redis goes away mid-run each later warmup is handed the same dead client and throws again until the process restarts. clearCircuitBreakerStoreOnRedisError existed for exactly this case but nothing ever called it. Wrap the Redis-backed store so a rejected operation clears the cached instance before rethrowing. The run that hit the outage still fails -- that one is lost either way -- but the next call re-probes Redis and falls back to SQLite while it is down. The three CircuitBreakerStore methods are wrapped one by one rather than proxied, since a missed one would silently keep the old behaviour. Releasing the client turned out to be the subtle half. retryStrategy returns null so a dropped client never reconnects on its own, yet its socket keeps the event loop open and every re-probe would add another one. Two details matter: The client is handed over for cleanup before connect() and ping() run, not after. Both can throw, and a client the catch block cannot see is a socket nobody ever closes -- the unreachable-Redis path leaked one on every call. Cleanup disconnects rather than quitting. quit() on a client that never finished connecting is queued until it is ready, which retryStrategy guarantees will never happen, so that promise never settles and anything chained to it to do the release never runs. disconnect() closes the socket whatever state it is in, and by this point the client has already been judged dead, so there is no reply worth draining. Tests stand up a Redis that completes the handshake and then misbehaves, which is the state the factory actually caches; an unreachable port only exercises the connect-time fallback that was already covered. The release test lives in its own file because an ioredis client left over from an earlier test in the same process wedges every later connect, so beside the routing tests it hangs rather than fails. Both were checked by reintroducing the defect and watching the test fail before restoring it. Signed-off-by: Minxi Hou <houminxi@gmail.com>
… them Both SQLite dual-writes in the Redis circuit-breaker store swallowed their error with a bare catch. The success-path one is the dangerous half: the Redis key carries a TTL, and once it is evicted the stale SQLite row is what remains, so a lost success write is how a connection ends up stuck in forbidden with nothing in the log to explain it. The forbidden-path write fails open rather than closed, which is milder, but it is still a lost write. Keep both non-fatal, since Redis is the source of truth for the live call, and warn with the connection id and a sanitized message. Signed-off-by: Minxi Hou <houminxi@gmail.com>
Two callers arriving before the cache is warm both saw a null storeInstance, both ran the Redis probe, and the second overwrote the first's client in the module-level slot. Nothing could close the overwritten one after that, so its socket stayed open and held the event loop. The window is narrow but reachable: the scheduler calls the factory once per tick, before its concurrency fan-out, and latches on the minute, so it takes a warmup cycle running past 60s for the next tick to overlap the previous one. That is exactly the situation where connections are already slow. Hold the in-flight probe and hand it to later callers instead of starting a second one. The reset helper drops it too, or a caller after a reset would be served the probe the reset was meant to discard. The test starts two calls without awaiting the first and asserts the server saw one connection and both callers got the same instance. It lives in its own file because a second ioredis case in the same process wedges every later connect -- reordering does not help, only a fresh process does. Signed-off-by: Minxi Hou <houminxi@gmail.com>
Contributor
Author
|
The failing checks here are inherited from the base, not from this diff. The run Happy to rebase onto the current tip if you would rather see a clean run before On priority: I have a follow-up branch ready that moves budget reset, token |
tests/unit/warmupScheduler/*.test.ts lived outside every runner glob
(test:unit only walks the explicit tests/unit/{api,...,lib,...}/**
allowlist), so the 6 warmup circuit-breaker tests never ran in CI.
Move them to tests/unit/lib/warmupScheduler/ to match the source
location (src/lib/warmupScheduler/) and fix up the relative import
depth so they are picked up by the existing tests/unit/lib/**
glob.
Co-authored-by: HouMinXi <HouMinXi@users.noreply.github.com>
|
UI for this? |
diegosouzapw
merged commit Aug 7, 2026
217ac4c
into
diegosouzapw:release/v3.8.50
4 of 5 checks passed
diegosouzapw
pushed a commit
that referenced
this pull request
Aug 8, 2026
Two files both claimed migration version 135: 135_connection_runtime_state.sql (#9449, landed 2026-08-07) and 135_migrate_model_capability_max_token.sql (#8908, landed 2026-08-05). #9449 branched before #8908 merged and never got renumbered before landing on release/v3.8.50. This is not cosmetic: getMigrationFiles() throws "Migration version collision detected" the moment ANY code path first touches the database (getDbInstance() -> runMigrations()), which means a completely fresh install/deploy from this branch cannot even boot — confirmed live against a freshly built container while testing unrelated live-verification tooling. Renumbered the later-landing file to 140 (the next free slot) and added the matching isSchemaAlreadyApplied("140") retroactive guard in migrationRunner.ts, so a DB that already ran this migration under the old 135 number isn't treated as needing a fresh application. This matches the established pattern already used for the prior 135/136 -> 137/138 renumber in the same file (also caused by the same recurring branch-before-merge numbering race). Test plan: - TDD: new tests/unit/migration-135-numbering-collision.test.ts (2/2) — spins up a hermetic fresh DB and confirms getDbInstance() applies every real on-disk migration without throwing, plus confirms both formerly-135 migrations' effects are present. Confirmed failing (reproducing the exact live crash) with the pre-fix colliding filenames restored, passing after the rename. - npm run typecheck:core — clean - npm run lint — clean - npm run check:file-size — clean (migrationRunner.ts rebaselined 1084->1094 for the new guard case) - Full migration-runner + migration-numbering test suites (64 tests across 6 files) — all pass, no regressions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Anthropic's 5-hour rolling window starts on the first inference request, so an account
that sits idle until 10am gives up an hour of capacity it could have had at 9am. #8848
proposed three ways to fix that; this implements Option B, the independent
warmupScheduler.tsthe issue itself favours over folding warmup into token refresh.A cron tick sends one
max_tokens: 1request to each opted-in Anthropic OAuthconnection, opening the window before real traffic arrives.
Off by default, and doubly gated. It needs
OMNIROUTE_WARMUP_ENABLEDtruthy andthe connection listed in
settings.claudeWarmup.connections. An empty connection listwarms nothing even with the env var on.
What a tick actually does:
America/Los_Angeles(Anthropic's reset zone) regardless of thehost clock. Default
0 7 * * *.claudeOAuth subscription connection —api_keyauth,terminal statuses (banned/expired/credits_exhausted), connections in backoff, and
connections already marked forbidden. Each skip logs its reason at debug level.
POST /v1/messageswithmax_tokens: 1and a rotating one-word prompt(
hi/hello/ping/ready), so repeat ticks are not byte-identical payloads.attempts for that connection; 429 parses
Retry-After.a SQLite fallback when Redis is unavailable. No hard Redis dependency is added.
Where this deviates from the issue
Three places, all worth your opinion rather than my assumption:
interval_minutes: 300plus anactive_hourswindow. This uses acron expression, which covers the issue's own "7am daily" example with less config
surface. Happy to switch to the interval form if you prefer it.
America/Los_Angelesrather than configurable. That isAnthropic's reset zone, so a server elsewhere still fires at the right moment — but it
is a deliberate narrowing of the issue's "server local time or configurable timezone".
scheduler skips terminal, backed-off and forbidden connections, but never inspects
remaining quota. One token against a near-limit account looked cheaper than a quota
lookup on every tick, though that is a judgement call I would rather you made.
Related Issues
Changes
src/lib/warmupScheduler.ts— the orchestrator: tick, gating, classification, retry.src/lib/warmupScheduler/{core,backoff,circuitBreakerStore,redisCircuitBreakerStore,sqliteCircuitBreakerStore,circuitBreakerFactory}.ts— types, backoff curve, the two stores and the factory that falls back between them.
src/lib/db/connectionRuntimeState.ts— column-level atomic UPSERT for warmup circuitstate, plus
src/lib/localDb.tsre-export.src/lib/db/migrations/135_connection_runtime_state.sql— new table only, no ALTER orbackfill of anything existing. Slot 134 is
134_proxy_logs_egress_ip.sqlon this base.src/shared/validation/settingsSchemas.ts,src/lib/db/settings.ts—claudeWarmupopt-in schema, defaulting to empty.
src/lib/initCloudSync.ts— mounts the scheduler afterstartBudgetResetJob..env.example+docs/reference/ENVIRONMENT.md— the fourOMNIROUTE_WARMUP_*vars,added in the same commit as the code that reads them.
Tests Added Or Updated
tests/unit/warmupScheduler.test.ts— orchestrator integration across each classifiedoutcome, not just the success path: opt-in gating, Bearer + beta headers, message
rotation, 401 refresh-and-retry, 403 forbidden persistence, 429
Retry-After,api_keyskip, terminal-status skip.tests/unit/warmupScheduler/redisCircuitBreakerStore.test.tstests/unit/warmupScheduler/sqliteCircuitBreakerStore.test.tstests/unit/warmupScheduler/circuitBreakerFactory.test.ts— Redis-to-SQLite fallback.tests/unit/warmupScheduler/backoff.test.ts— curve and cap.tests/unit/db/connectionRuntimeState.test.ts— including the concurrent-write casethe atomic UPSERT exists for.
Verification run locally:
All three commits were also walked individually through
check:migration-numbering,check:db-rulesandcheck:cycles, so the historybisects clean rather than only being green at the tip.
Coverage Notes
Every new module under
src/lib/warmupScheduler/has a matching test file, and theorchestrator is driven through a stubbed fetch across each outcome it classifies.
One deliberate gap:
initCloudSync.tsgains a singlestartWarmupScheduler()call andhas no new test, since the existing bootstrap has no test harness.
Reviewer Notes
connectionRuntimeState.tsuses a column-level UPSERT rather thanread-modify-write. The tick runs up to 10 connections at a time, so two concurrent
warmups would otherwise clobber each other's circuit state.
in Redis but left the SQLite backup's
last_warmup_resultatforbidden, so a Rediseviction would resurrect a stale block.
clearWarmupCircuitdoes not touch thatcolumn, hence routing through
upsertWarmupStateinstead. The test asserts the SQLitebackup's final state, verified by injecting the regression and watching it fail first.
one per connection per day.
check:file-sizecurrently reportssrc/sse/handlers/chat.tsandopen-sse/executors/base.tsover their frozen baselines. Neither file is in thisdiff, and both are byte-identical to
release/v3.8.50, so those two are pre-existingon the base rather than anything this PR introduces.