Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/api/src/routes/v2/v2-test-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions apps/api/src/services/warehouse/QueryEngineService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ export class QueryEngineService extends Context.Service<QueryEngineService, Quer
response.result.kind === "timeseries" ? response.result.data : [],
),
),
// Resolve this org's warehouse route once before the fill fans
// out. Without it each branch resolves it independently and they
// all miss the in-isolate memo, because they start together:
// measured in prod as the same config read running twice
// concurrently at 2.90s each, against warehouse queries of 428ms
// and 1179ms. The bucket cache only runs this on a >1-range fill,
// so cache hits and single-range fills are unaffected.
warehouse.warmRoute(tenant),
)

yield* Metric.update(QueryEngineMetrics.bucketCacheBucketsHit, outcome.bucketsHit)
Expand Down
26 changes: 26 additions & 0 deletions packages/query-engine/src/caching/bucket-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,25 @@ export interface BucketCacheServiceShape {
readonly getOrComputeBuckets: <E, R>(
request: BucketCacheRequest,
computeRange: (range: TimeRange) => Effect.Effect<ReadonlyArray<TimeseriesPoint>, 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<void>,
) => Effect.Effect<BucketCacheOutcome, E, R>
}

Expand Down Expand Up @@ -431,6 +450,7 @@ export class BucketCacheService extends Context.Service<BucketCacheService, Buck
const getOrComputeBuckets = Effect.fn("BucketCacheService.getOrComputeBuckets")(function* <E, R>(
request: BucketCacheRequest,
computeRange: (range: TimeRange) => Effect.Effect<ReadonlyArray<TimeseriesPoint>, E, R>,
prepare?: Effect.Effect<void>,
) {
const bucketMs = request.bucketSeconds * 1000
const segmentMs = bucketMs * segmentBucketCount
Expand Down Expand Up @@ -536,6 +556,12 @@ export class BucketCacheService extends Context.Service<BucketCacheService, Buck
fluxBoundaryMs,
)
const fillRanges = coalesceMissingRanges(missing)
// One warm-up before the fan-out, not one route resolution per
// branch. See `prepare` on BucketCacheServiceShape for the trace
// this came from. Only worth it above one range — see the doc there.
if (prepare !== undefined && fillRanges.length > 1) {
yield* prepare
}
const freshByRange = yield* Effect.forEach(
fillRanges,
(item) => computeRange(item.range),
Expand Down
11 changes: 11 additions & 0 deletions packages/query-engine/src/execution/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions packages/query-engine/src/execution/ports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,24 @@ export interface WarehouseQueryServiceShape {
compiled: CompiledQuery<T> | ((capabilities: WarehouseCapabilities) => CompiledQuery<T>),
options?: SqlQueryOptions,
) => Effect.Effect<Option.Option<T>, 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<void>
readonly ingest: <T>(
tenant: ExecutionTenant,
datasource: string,
Expand Down
Loading