From 8f4c9c047af4f91383b5eecb1ce13137129f05b1 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Wed, 5 Aug 2026 00:18:03 +0200 Subject: [PATCH] perf(warehouse): resolve org route once per bucket-cache fan-out 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. --- apps/api/src/routes/v2/v2-test-support.ts | 3 +++ .../warehouse/QueryEngineCacheHitRate.test.ts | 3 +++ .../QueryEngineEvaluateCache.test.ts | 3 +++ .../services/warehouse/QueryEngineService.ts | 8 ++++++ .../query-engine/src/caching/bucket-cache.ts | 26 +++++++++++++++++++ .../query-engine/src/execution/executor.ts | 11 ++++++++ packages/query-engine/src/execution/ports.ts | 18 +++++++++++++ 7 files changed, 72 insertions(+) diff --git a/apps/api/src/routes/v2/v2-test-support.ts b/apps/api/src/routes/v2/v2-test-support.ts index 65ddfc9dc..09ce58af1 100644 --- a/apps/api/src/routes/v2/v2-test-support.ts +++ b/apps/api/src/routes/v2/v2-test-support.ts @@ -151,6 +151,9 @@ export const WarehouseServiceStubLayer = Layer.succeed(WarehouseQueryService, { compiledQueryBounded: die, compiledQueryWithCapabilities: die, compiledQueryFirst: die, + // Not `die`: warming is best-effort and silent by contract, so a stub that + // throws would fail a path that only tried to warm up. + warmRoute: () => Effect.void, ingest: die, asExecutor: dieSync, }) diff --git a/apps/api/src/services/warehouse/QueryEngineCacheHitRate.test.ts b/apps/api/src/services/warehouse/QueryEngineCacheHitRate.test.ts index e97f1c603..029301ae0 100644 --- a/apps/api/src/services/warehouse/QueryEngineCacheHitRate.test.ts +++ b/apps/api/src/services/warehouse/QueryEngineCacheHitRate.test.ts @@ -127,6 +127,9 @@ const makeStub = (counter: { n: number }): WarehouseQueryServiceShape => counter.n += 1 return compiled.decodeFirstRow(ROWS).pipe(Effect.orDie) }, + // Deliberately does not touch `counter`: warming resolves route config, it + // does not issue a warehouse query, and these tests assert query counts. + warmRoute: () => Effect.void, ingest: () => Effect.void, sql: () => Promise.resolve({ data: [] }), }) as unknown as WarehouseQueryServiceShape diff --git a/apps/api/src/services/warehouse/QueryEngineEvaluateCache.test.ts b/apps/api/src/services/warehouse/QueryEngineEvaluateCache.test.ts index 3885419d0..0b8474f6d 100644 --- a/apps/api/src/services/warehouse/QueryEngineEvaluateCache.test.ts +++ b/apps/api/src/services/warehouse/QueryEngineEvaluateCache.test.ts @@ -300,6 +300,9 @@ const makeFullStub = ( counter.n += 1 return compiled.decodeFirstRow(rows).pipe(Effect.orDie) }, + // Deliberately does not touch `counter`: warming resolves route config, it + // does not issue a warehouse query, and these tests assert query counts. + warmRoute: () => Effect.void, ingest: () => Effect.void, sql: () => Promise.resolve({ data: [] }), }) as unknown as WarehouseQueryServiceShape diff --git a/apps/api/src/services/warehouse/QueryEngineService.ts b/apps/api/src/services/warehouse/QueryEngineService.ts index 224ce7a3a..96d6ad852 100644 --- a/apps/api/src/services/warehouse/QueryEngineService.ts +++ b/apps/api/src/services/warehouse/QueryEngineService.ts @@ -196,6 +196,14 @@ export class QueryEngineService extends Context.Service1-range fill, + // so cache hits and single-range fills are unaffected. + warehouse.warmRoute(tenant), ) yield* Metric.update(QueryEngineMetrics.bucketCacheBucketsHit, outcome.bucketsHit) diff --git a/packages/query-engine/src/caching/bucket-cache.ts b/packages/query-engine/src/caching/bucket-cache.ts index b82c816ce..ceeaaeed4 100644 --- a/packages/query-engine/src/caching/bucket-cache.ts +++ b/packages/query-engine/src/caching/bucket-cache.ts @@ -382,6 +382,25 @@ export interface BucketCacheServiceShape { readonly getOrComputeBuckets: ( request: BucketCacheRequest, computeRange: (range: TimeRange) => Effect.Effect, E, R>, + /** + * Optional warm-up run once before a multi-range fill fans out. + * + * Each `computeRange` branch resolves the tenant's warehouse route on its + * own, and that resolution reads per-org config from Postgres (~2.9s + * cold). Started together, every branch misses the in-isolate memo and + * pays it: one prod trace resolved the identical config twice + * concurrently at 2.90s each while the queries being prepared for took + * 428ms and 1179ms. Running this first collapses that to one lookup. + * + * Deliberately NOT part of `BucketCacheRequest` — that object is + * canonicalized into the cache-key fingerprint, and an Effect in it would + * poison the key. + * + * Skipped when the fill is 0 or 1 ranges: with none there is nothing to + * prepare, and with one the warm-up would move the same cost rather than + * remove it, while adding a sequential step to a pure cache hit. + */ + prepare?: Effect.Effect, ) => Effect.Effect } @@ -431,6 +450,7 @@ export class BucketCacheService extends Context.Service( request: BucketCacheRequest, computeRange: (range: TimeRange) => Effect.Effect, E, R>, + prepare?: Effect.Effect, ) { const bucketMs = request.bucketSeconds * 1000 const segmentMs = bucketMs * segmentBucketCount @@ -536,6 +556,12 @@ export class BucketCacheService extends Context.Service 1) { + yield* prepare + } const freshByRange = yield* Effect.forEach( fillRanges, (item) => computeRange(item.range), diff --git a/packages/query-engine/src/execution/executor.ts b/packages/query-engine/src/execution/executor.ts index f9e2d4850..77baca70c 100644 --- a/packages/query-engine/src/execution/executor.ts +++ b/packages/query-engine/src/execution/executor.ts @@ -1006,6 +1006,17 @@ WHERE name = 'enable_full_text_index'`, compiledQueryWithCapabilities, compiledQueryFirst: (tenant, compiled, options) => unbounded(compiledQueryFirst(tenant, compiled, withoutResponseLimits(options))), + // `resolveCapabilities` resolves the route on its way through, so warming + // it warms both. `ignore` keeps a failed warm-up invisible — the real + // query behind it fails with its own context a moment later. + warmRoute: (tenant, options) => + resolveCapabilities(tenant, options).pipe( + Effect.asVoid, + Effect.ignore, + Effect.withSpan("WarehouseQueryService.warmRoute", { + attributes: { orgId: tenant.orgId }, + }), + ), ingest, asExecutor, } satisfies WarehouseQueryServiceShape diff --git a/packages/query-engine/src/execution/ports.ts b/packages/query-engine/src/execution/ports.ts index a856081cb..f3ba800df 100644 --- a/packages/query-engine/src/execution/ports.ts +++ b/packages/query-engine/src/execution/ports.ts @@ -144,6 +144,24 @@ export interface WarehouseQueryServiceShape { compiled: CompiledQuery | ((capabilities: WarehouseCapabilities) => CompiledQuery), options?: SqlQueryOptions, ) => Effect.Effect, WarehouseSqlError | WarehouseValidationError> + /** + * Resolve this tenant's route and capabilities once, so a fan-out that + * follows finds them memoized instead of each branch deriving them itself. + * + * Exists because route resolution reads per-org ClickHouse config from + * Postgres, and that read has been measured at ~2.9s cold. A fan-out that + * starts every branch at once has every branch miss the in-isolate memo: + * one prod trace of a single dashboard panel resolved the identical config + * twice concurrently at 2.90s each, while the two warehouse queries the + * fan-out existed to run took 428ms and 1179ms. The lookup cost more than + * double the work it was preparing for. + * + * Cheap and idempotent on a warm memo, so callers may invoke it + * unconditionally. Errors are swallowed: this is a warm-up, and the real + * query behind it reports failures with proper context. Never let this + * change the error semantics of the path it precedes. + */ + readonly warmRoute: (tenant: ExecutionTenant, options?: SqlQueryOptions) => Effect.Effect readonly ingest: ( tenant: ExecutionTenant, datasource: string,