perf(api): serve the org warehouse config from a durable KV tier - #387
Merged
Conversation
The SWR memo shipped in 445d3ee only helps an isolate that has already paid once, and Workers evict isolates constantly. Measured over the three days since it landed, `resolveCachedSettings` still fell through to Postgres 10-13k times/day at a p50 of 482-636ms each -- 1.4-2.6 hours of blocked wall time per day, on a read that sits in front of every widget query on a dashboard. That cost is the per-`.execute()` postgres.js dial, not the query; the "~26ms Postgres read" in the old comment was the edge-cache read p50, not this. Only a shared cache removes it, since the whole point is to help an isolate that has never seen the org. Put that tier on Workers KV rather than `caches.default`. The Cache API is not slow -- a completed `cache.match()` is 7ms p50 -- but it holds one of the Worker's six simultaneous-connection slots while awaiting headers and is not cancellable, so the read deadline abandons the wait but not the slot and `compute` queues behind a seventh connection. Still true on 2026-08-10 across the remaining buckets: 15% of reads time out, p95 16.4s, 1003s blocked in a day. A KV `get` is an ordinary subrequest, so abandoning one is closer to actually free. That last point is a hypothesis, not a measurement. `cache.backend` is on every getOrCompute span so the two can be compared; if KV contends for the same six slots it will show the same bimodal split, and dropping the EDGE_CACHE binding reverts to the Cache API with no code change. - CacheBackend gains a KV implementation, selected when EDGE_CACHE is bound. Swaps the backend for every bucket, so qe-direct / qe-execute / autumn-customer -- which show the same pathology -- get measured too. - The config read goes through EdgeCacheService between memo and Postgres, resolved with `Effect.serviceOption` so tests and unbound hosts degrade to a direct read. `readTimeoutMs` is 150ms, not the 40ms default, because that default assumes a cheap `compute` and here `compute` is the ~500ms dial. - Invalidation becomes load-bearing: a missed memo bust self-healed in minutes, a missed KV bust is an hour of the whole fleet reading a config that no longer exists. `invalidateOrgRuntimeConfig` drops both tiers and every writer calls it, including both schema-apply workflow writes (they stamp `schema_version`, which is in the cached projection). - Raise the memo hard ceiling 15min -> 6h. It is an isolate-lifetime backstop, not a staleness bound -- the soft TTL is -- and at 15min it made a bursty widget fan-out block at the head of every burst. KV returns null for a missing key while EdgeCacheBackend uses undefined for "not cached", and a cached null is a real value -- on this bucket the common one ("managed org, use Tinybird"). Values are stored wrapped as `{ v }`; a bare null would have made every managed org a permanent miss. The memory backend cannot catch that class of bug because it stores raw values, so CacheBackendLive.test.ts covers it directly. Scoped to apps/api. apps/alerting has no KV binding and still uses `caches.default`; sharing one namespace across two alchemy stacks needs an ID reference rather than a second create.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Two CI failures from the KV tier, both invisible locally because I only
ran targeted files.
`WorkerEnvironment` destructures `env` off the `cloudflare:workers`
module, but the vitest stub only exported `DurableObject` and
`WorkflowEntrypoint`. Every consumer so far layered its bindings in
rather than reading them off the service, so nothing had dereferenced
the result before; `CacheBackendLive` reading `env.EDGE_CACHE` is the
first, and it crashed 10 alerts.http tests with "Cannot read properties
of undefined". Export `env` from the stub so the double matches the real
module, and make `WorkerEnvironment.layer` fall back to `{}` so a host
without one hands out an empty binding record rather than `undefined`
behind a non-nullable type.
The SWR refresh assertions also went flaky. A forked refresh completes
when real promises settle -- a PGlite read, plus a durable-tier read
whose key hashing goes through `crypto.subtle` -- and none of that is
driven by `TestClock`, so `adjust(1)` was never a guarantee. Adding the
durable tier lengthened that chain enough for a slower CI machine to
lose the race. Poll for the refreshed value with `resolveUntilUrl`
instead, bounded so a refresh that never lands still fails. The stale
assertion stays exactly where it was, so "nobody blocked" is still
asserted on the nose.
Two findings from review, both real. `apps/alerting` runs OrgClickHouseSettingsService too, but it has no EDGE_CACHE binding, so its CacheBackend is `caches.default` -- a store no `invalidateOrgRuntimeConfig` ever reaches, because every writer lives in apps/api and busts KV. An hour-long entry there would let alert evaluation keep using a warehouse config the customer already rotated or deleted: firing on stale data, or missing incidents outright. My PR description waved this away on the grounds that alerting never writes the config, which was the wrong question -- the hazard is that it reads from a store the writers cannot bust. Expose `backendName` on EdgeCacheServiceShape and use the durable tier only where invalidation lands: `workers-kv` (the binding apps/api owns, where the busts go) and `memory` (per-isolate, self-consistent by construction). A `workers-cache` host falls back to memo + Postgres -- slower, and right. The proper fix is binding the same namespace into the alerting stack, which already binds Hyperdrive by ID across stacks; until then the tier stays off there rather than silently serving stale routing. Second: KV refuses an `expirationTtl` under 60s and the backend clamped upward, but real buckets ask for less -- `qe-evaluate` and the integrations routes use 30s. The other two backends carry their own deadline (`Cache-Control: max-age`, `expiresAt`), so a 30s entry was being served for 60s. Carry the real deadline in the envelope as `exp` and expire on read; `expirationTtl` still goes out at the floor to bound storage.
🍁 Maple PR previewNote Preview resources were removed when this pull request closed. Final commit |
Makisuo
added a commit
that referenced
this pull request
Aug 10, 2026
#387 put the org warehouse config on a durable KV tier, on the hypothesis that a KV `get` is a cancellable subrequest and so cheaper to abandon than an uncancellable `cache.match()`. `makeKvBackend` said to treat it as an experiment until `cache.read_status` per `cache.backend` said otherwise. It does. Measured over 24h on the live deploy: - KV reads that COMPLETE take 92ms (vs 6ms on the Cache API), and 79% of them hit their deadline anyway. - It was bound where the cost isn't. 94% of the Postgres fallback it targets is apps/alerting (4,646 resolutions/day at p50 573ms, ~44min of blocked wall time), which has no KV binding and so could never use the tier. apps/api, which had it, falls back at p50 24ms — cheaper than a KV read. - It regressed three unrelated buckets: CacheBackendLive returned KV whenever EDGE_CACHE was bound, so qe-direct, qe-execute and autumn-customer flipped to KV at the 40ms service default — below KV's ~92ms floor, so they time out 100% of the time at span p50 9.0s. The reason this and the earlier Cache API attempt both failed is that the cost is not a cold-isolate miss. Grouping alerting's Postgres resolutions by trace: 1,033 traces do zero, while 106 traces do 22 each — half of all of them. It is an in-request fan-out where every branch misses the memo because none has finished writing it yet. No shared cache fixes concurrent siblings; it just turns N Postgres reads into N cache reads contending for the same six connection slots. The fix is to resolve the config once before the fan-out, the way `warehouse.warmRoute` already does in query-engine.http.ts — follow-up, not in this change. With KV gone the durable tier is dead in prod (durableTierIsInvalidated would admit only "memory"), so it is removed entirely rather than left alive only under test. The memo + Postgres path is unchanged, and it is strictly more correct: no entry can outlive an invalidation.
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
1 participant
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.
Why
OrgClickHouseSettingsService.resolveCachedSettingsruns on the hot path of every warehouse SQL execution, and the bucket-cache fan-out re-runs it once per missing range. The SWR memo from445d3ee9conly helps an isolate that has already paid once — and Workers evict isolates constantly.Prod, the three days since that memo landed:
clickhouse.config.sourcememomemo_stalepostgres~4.5% of resolutions still cost half a second each — 1.4–2.6 hours of blocked wall time per day. That cost is the per-
.execute()postgres.js dial, not the query. (The "~26ms Postgres read" in the old comment was the edge-cache read p50, not this one.)Only a shared cache removes it: the whole point is to help an isolate that has never seen the org.
Why KV and not
caches.defaultThe Cache API is not slow — a completed
cache.match()is 7ms p50. Every read inmaple-apion 2026-08-10:cache.read_statushitmisstimeout15% of reads never complete.
cache.match()holds one of the Worker's six simultaneous-connection slots while awaiting headers and is not cancellable, so the 40ms deadline abandons the wait but not the slot —computethen opens a seventh connection and queues behind the read it just gave up on. That's thetimeoutp50 of 40ms (deadline fired on time) next to a p95 of 16.4s (fallback starved). It's why the tier was removed from this path in the first place.A KV
getis an ordinary subrequest, so abandoning one is closer to actually free.This is a hypothesis, not a measurement. If KV contends for the same six slots it will show the same bimodal split.
cache.backendis on everygetOrComputespan so the two are directly comparable, and dropping theEDGE_CACHEbinding reverts to the Cache API with no code change.What changed
CacheBackendgains a KV implementation (CacheBackendLive.ts), selected whenEDGE_CACHEis bound. This swaps the backend for every bucket, soqe-direct/qe-execute/autumn-customer— which show the same pathology — get measured too.EdgeCacheServicebetween memo and Postgres, resolved withEffect.serviceOptionso tests and unbound hosts degrade to a direct read.readTimeoutMsis 150ms rather than the 40ms default: that default assumes a cheapcompute, and herecomputeis the ~500ms dial.invalidateOrgRuntimeConfigdrops both tiers, and every writer calls it — including both schema-apply workflow writes, which stampschema_version(part of the cached projection).Reviewer notes
The bug worth looking at. KV returns
nullfor a missing key, whileEdgeCacheBackendusesundefinedfor "not cached" — and a cachednullis a real value here, in fact the common one ("managed org, use Tinybird"). A barenullwrite would have made every managed org a permanent miss and sent it to Postgres forever. Values are stored wrapped as{ v }. The memory backend can't catch this class of bug because it stores raw values, soCacheBackendLive.test.tstests the KV backend directly.Test semantics changed. Four existing tests failed on the first run: they used a raw
UPDATE org_clickhouse_settingsas the instrument for observing memo mechanics, and a raw write is now invisible to the durable tier too. They now call anevictDurablehelper so they still test the memo rather than silently asserting nothing. The same trap will catch the next test written against this path.Scope.
apps/apionly.apps/alertinghas no KV binding and still usescaches.default— sharing one namespace across two alchemy stacks needs an ID reference rather than a second create, which I didn't want to do silently. Alerting reads this config but never writes it, so there's no invalidation correctness issue.Deploy note. Adds an
EDGE_CACHEKV namespace binding (wrangler.jsonc+alchemy.run.ts).Verification
bunx vitest run CacheBackendLive OrgClickHouseSettingsService— 29 passedbunx vitest run WarehouseQueryService ClickHouseSchemaApplyWorkflow— passedlib/cachesuite — 17 passedbun typecheck— cleanPost-deploy, before trusting this: group
cache.read_statusbycache.backend. Ifworkers-kvshows atimeoutrate anything like the Cache API's 15%, pull the binding. Then confirmclickhouse.config.source = "postgres"collapses toward zero.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.