Skip to content

perf(warehouse): resolve org route once per bucket-cache fan-out - #343

Merged
Makisuo merged 1 commit into
mainfrom
perf/dedupe-warehouse-route-resolution
Aug 4, 2026
Merged

perf(warehouse): resolve org route once per bucket-cache fan-out#343
Makisuo merged 1 commit into
mainfrom
perf/dedupe-warehouse-route-resolution

Conversation

@Makisuo

@Makisuo Makisuo commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What

BucketCacheService.getOrComputeBuckets fans out computeRange at fillConcurrency, and every branch independently resolved the same org warehouse route — including the Postgres read behind it. This resolves it once before the fan-out instead.

Why

Found in prod trace 625a0740fffa7fbeecdcf2c03bcda4ea — a 5.21s dashboard panel:

POST /api/query-engine/execute ......................... 4.17s
└─ BucketCacheService.readOrCompute .................... 4.17s
   ├─ EdgeCache.rawGetDetailed ........................ 40ms   (miss)
   ├─ QueryEngineService.execute ...................... 3.37s  ┐ two fill branches,
   │  ├─ resolveRoute → SELECT org_clickhouse_settings  2.90s  │ started together
   │  └─ executeSql  (the actual query) ............... 428ms  │
   └─ QueryEngineService.execute ...................... 4.13s  │
      ├─ resolveRoute → SELECT org_clickhouse_settings  2.90s  │ ← identical lookup
      └─ executeSql  (the actual query) .............. 1179ms  ┘

The identical config lookup ran twice, concurrently, at 2.90s each — more than double the cost of the two warehouse queries it was preparing for.

All branches start before any finishes, so all of them miss the 300s runtimeConfigMemo. It's a thundering herd inside a single request. The "no single-flight" rule in EdgeCacheService is about cross-request sharing (Cloudflare ties I/O objects to the request that created them) — it never applied within one request.

Supporting measurements across 2 days of prod traces:

layer n p50 p95
WarehouseQueryService.executeSql (actual SQL) 4343 192ms 3437ms
SELECT org_clickhouse_settings (maple-api) 398 2322ms 4507ms
SELECT org_ingest_keys (same DB, same driver) 3261 15ms 26ms

The queries are not the bottleneck; the metadata lookup beside them is.

How

  • warmRoute added to WarehouseQueryServiceShape — resolves route + capabilities once. Effect.ignored, so a failed warm-up can never change the error semantics of the path it precedes; the real query behind it still reports failures with proper context.
  • prepare — optional third arg to getOrComputeBuckets, run only when fillRanges.length > 1. With zero ranges there's nothing to prepare; with one, the warm-up would move the cost rather than remove it while adding a sequential step to what may be a pure cache hit.

Reviewer notes

  • prepare is a separate parameter, not a field on BucketCacheRequest — that object is canonicalized into the cache-key fingerprint, so an Effect in it would poison the key.
  • The three warehouse test doubles used as unknown as WarehouseQueryServiceShape. The cast hid the missing method from tsc; only the runtime caught it. Worth knowing those stubs don't type-check against the real shape.
  • Their warmRoute stubs deliberately don't touch the call counters — warming resolves config, it doesn't issue a warehouse query, and those tests assert query counts.
  • Warehouse queries are still fanned out in parallel; only the route resolution is serialized ahead of them.

Testing

  • bun run --cwd packages/query-engine typecheck + test — 989 pass
  • bun run --cwd apps/api typecheck
  • bunx vitest run src/services/warehouse/ — 130 pass, 126 skipped (ClickHouse e2e, needs ch:up)

Not yet verified in prod. After deploy, confirm resolveRuntimeConfig appears once per request rather than once per fill branch.

Follow-ups (not in this PR)

  • executeQueryBuilder runs QueryEngineService.execute at concurrency 4 and almost certainly has the same duplicate-resolution shape.
  • The 2.9s itself is still unexplained — a primary-key lookup on a table in the same AWS region should be ~5ms. This removes one of two; the remaining one is still 2.9s.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

A prod trace of a 5.21s dashboard panel showed BucketCacheService fanning out
two computeRange branches that each independently ran
resolveCapabilities -> resolveRoute -> resolveRuntimeConfig ->
SELECT org_clickhouse_settings at 2.90s, concurrently, while the two warehouse
queries they existed to prepare for took 428ms and 1179ms. The config lookup
cost more than double the work it was setting up.

All branches start before any finishes, so all of them miss the 300s
runtimeConfigMemo -- a thundering herd inside a single request. The
"no single-flight" rule in EdgeCacheService is about CROSS-request sharing
(Cloudflare ties I/O objects to the request that created them) and never
applied within one request.

Adds warmRoute to WarehouseQueryServiceShape, which resolves route and
capabilities once. It is Effect.ignore'd so a failed warm-up can never change
the error semantics of the path it precedes -- the real query behind it still
reports failures with proper context.

BucketCacheService.getOrComputeBuckets takes an optional `prepare` effect, run
only when fillRanges.length > 1: with zero ranges there is nothing to prepare,
and with one the warm-up would move the cost rather than remove it while adding
a sequential step to what may be a pure cache hit. It is a separate parameter
rather than a field on BucketCacheRequest because that object is canonicalized
into the cache-key fingerprint, where an Effect would poison the key.

The three warehouse test doubles used `as unknown as WarehouseQueryServiceShape`,
so the cast hid the missing method from tsc and only the runtime caught it.
Their warmRoute stubs deliberately do not touch the call counters: warming
resolves config, it does not issue a warehouse query, and those tests assert
query counts.

Verified: packages/query-engine typecheck + 989 tests; apps/api typecheck;
apps/api/src/services/warehouse 130 tests. Not yet verified in prod.
@Makisuo
Makisuo merged commit 550bf40 into main Aug 4, 2026
17 checks passed
@Makisuo
Makisuo deleted the perf/dedupe-warehouse-route-resolution branch August 4, 2026 22:31
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🍁 Maple PR preview

Note

Preview resources were removed when this pull request closed.

Final commit 8f4c9c0 · View workflow run

Makisuo added a commit that referenced this pull request Aug 5, 2026
…cket cache

Prod measurement of EdgeCacheService.getOrCompute on the org-clickhouse-config
bucket, over 1039 reads:

  hit      597   p50    8ms
  miss      64   p50   27ms   <- includes the actual Postgres read
  timeout  378   p50 2650ms

A cold Postgres read costs 27ms. The ~2.9s that has been attributed to it all
along is the cache read being ABANDONED at its 40ms deadline, which happens on
36% of reads. That rate matches the failure mode already documented in
edge-cache.ts almost exactly (35.9% measured at 4 reads/request): a
cache.match() issued while a sibling branch's warehouse fetch holds a
connection slot gets queued and never returns in time.

So the fix is ordering, not caching. #343 added warmRoute but only called it
from the bucket-cache fill path, and only when the fill split into more than one
range. Every other fan-out -- the service bundles, the Cloudflare and
PlanetScale panels, listPods, serviceDbQuerySummary -- still issued its config
read concurrently with sibling warehouse fetches.

Calling warmRoute immediately before each of the 14 Effect.all sites means the
cache read happens with an empty connection pool, so it lands in ~8ms; every
branch behind it then hits the in-isolate memo for free. On a warm memo the call
is a no-op, so it costs nothing when there is nothing to warm.

Verified: apps/api typecheck. Tests not run locally by request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant