diff --git a/package-lock.json b/package-lock.json index 553f20b..37ef7f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,7 @@ "@libpg-query/parser": "^17.6.3", "@opentelemetry/api": "^1.9.0", "@pgsql/types": "^17.6.2", - "@query-doctor/core": "^0.17.0", + "@query-doctor/core": "^0.18.0", "async-sema": "^3.1.1", "capnweb": "^0.7.0", "dedent": "^1.7.1", @@ -1193,9 +1193,9 @@ "license": "BSD-3-Clause" }, "node_modules/@query-doctor/core": { - "version": "0.17.0", - "resolved": "https://registry.npmjs.org/@query-doctor/core/-/core-0.17.0.tgz", - "integrity": "sha512-WN0so5G6dqmOx0v7NGw58nTbTlIXdX/eTHRIyxSP/uBLPGfYPmTna3B+o4s2KixA7uVez5uaCM//1KhJ36r/Ww==", + "version": "0.18.0", + "resolved": "https://registry.npmjs.org/@query-doctor/core/-/core-0.18.0.tgz", + "integrity": "sha512-Z3ldmkUv92U2vxw9OI8J4eOn/MUxTMheqcRy+HJuCeMAcZGj2Sbv4XB60TzhtnLIbMt4EBCK/NysLeGxjlWm8w==", "dependencies": { "@pgsql/types": "^17.6.2", "capnweb": "^0.10.0", diff --git a/package.json b/package.json index 2bd0eb9..c788625 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "@libpg-query/parser": "^17.6.3", "@opentelemetry/api": "^1.9.0", "@pgsql/types": "^17.6.2", - "@query-doctor/core": "^0.17.0", + "@query-doctor/core": "^0.18.0", "async-sema": "^3.1.1", "capnweb": "^0.7.0", "dedent": "^1.7.1", diff --git a/src/config.test.ts b/src/config.test.ts index b57a888..b1c943a 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -1,5 +1,5 @@ import { test, expect, vi } from "vitest"; -import { DEFAULT_CONFIG } from "./config.ts"; +import { configuredStatisticsScale, DEFAULT_CONFIG } from "./config.ts"; import type { ServerApi } from "@query-doctor/core"; import type { RpcStub } from "capnweb"; @@ -55,3 +55,19 @@ test("passes repo and branch to getRepoConfig", async () => { await resolveConfig(api, "org/repo", "feat/my-branch"); expect(getRepoConfig).toHaveBeenCalledWith("org/repo", "feat/my-branch"); }); + +test("reads the scale the repo is configured at", () => { + expect(configuredStatisticsScale({ ...DEFAULT_CONFIG, statisticsScale: 10 })) + .toBe(10); +}); + +test("falls back to the current data size when the repo config has no scale", () => { + // A Site that predates the setting, or a run with no repo config at all. + expect(configuredStatisticsScale(DEFAULT_CONFIG)).toBe(1); + expect(configuredStatisticsScale(undefined)).toBe(1); +}); + +test("ignores a scale the Site could not have meant", () => { + expect(configuredStatisticsScale({ statisticsScale: 0 })).toBe(1); + expect(configuredStatisticsScale({ statisticsScale: "10" })).toBe(1); +}); diff --git a/src/config.ts b/src/config.ts index bfdc801..76a7959 100644 --- a/src/config.ts +++ b/src/config.ts @@ -13,6 +13,12 @@ export interface AnalyzerConfig { * it through `getRepoConfig`; the gate honours it the moment it arrives. */ conditionPolicies?: RepoPolicyConfig; + /** + * The multiple of the current data the repo asks its queries to be planned + * against (#3119). 1 is the current size, which is what a repo gets until + * someone raises it in CI settings. + */ + statisticsScale?: number; } export const DEFAULT_CONFIG: AnalyzerConfig = { @@ -20,4 +26,21 @@ export const DEFAULT_CONFIG: AnalyzerConfig = { regressionThreshold: 0, ignoredQueryHashes: [], acknowledgedQueryHashes: [], + statisticsScale: 1, }; + +/** + * The scale a repo is configured at, read off whatever the relay returned. + * + * The Site has sent `statisticsScale` on the repo config since #3115, but + * core's `RepoConfig` type does not declare it, so the read is defensive and + * lives in this one place. A missing, non-numeric, or sub-1 value means the + * current data size: an analyzer talking to an older Site keeps working, and a + * nonsense value never reaches the planner. Drop the cast once core ships the + * field on `RepoConfig`. + */ +export function configuredStatisticsScale(config: unknown): number { + const scale = (config as { statisticsScale?: unknown } | null | undefined) + ?.statisticsScale; + return typeof scale === "number" && scale >= 1 ? scale : 1; +} diff --git a/src/main.ts b/src/main.ts index 33b526e..4cf1f77 100644 --- a/src/main.ts +++ b/src/main.ts @@ -21,7 +21,7 @@ import { gateRegression } from "./gate/regression.ts"; import { gateNewQuery } from "./gate/new-query.ts"; import { gateSchemaChange } from "./gate/schema-change.ts"; import { resolveVerdict } from "./gate/policy.ts"; -import { DEFAULT_CONFIG } from "./config.ts"; +import { configuredStatisticsScale, DEFAULT_CONFIG } from "./config.ts"; import { ApiClient } from "./remote/api-client.ts"; import { Remote } from "./remote/remote.ts"; import { ConnectionManager } from "./sync/connection-manager.ts"; @@ -99,6 +99,14 @@ async function runInCI( ? new PgbadgerSource(logPath) : remoteDbManager.getConnectorFor(sourcePostgresUrl); + const statisticsScale = configuredStatisticsScale(config); + if (statisticsScale !== 1) { + log.info( + `Modeling queries at ${statisticsScale}x the current data size`, + "main", + ); + } + const runner = await Runner.build({ targetPostgresUrl, sourcePostgresUrl, @@ -107,6 +115,7 @@ async function runInCI( ignoredQueryHashes: config.ignoredQueryHashes, remote, productionStats: productionStats ?? undefined, + statisticsScale, }); let allResults: QueryProcessResult[]; let reportContext; diff --git a/src/runner.test.ts b/src/runner.test.ts index dacb7f0..28ffc5e 100644 --- a/src/runner.test.ts +++ b/src/runner.test.ts @@ -80,3 +80,53 @@ describe("Runner.determineStatsMode precedence", () => { expect(await Runner.determineStatsMode()).toEqual(syntheticMode); }); }); + +describe("Runner.determineStatsMode at a configured scale", () => { + const TABLE: ExportedStats = { + tableName: "users", + schemaName: "public", + relpages: 10, + reltuples: 166_000, + relallvisible: 8, + columns: [], + indexes: [], + }; + + /** CI always resolves a static mode; this narrows the union to read it. */ + async function staticMode( + productionStats?: ExportedStats[], + scale?: number, + ) { + const strategy = await Runner.determineStatsMode( + undefined, + productionStats, + scale, + ); + if (strategy.type !== "static") { + throw new Error(`expected a static stats mode, got ${strategy.type}`); + } + return strategy.stats; + } + + test("models production stats at the repo's scale", async () => { + const stats = await staticMode([TABLE], 10); + + expect(stats.kind).toBe("fromStatisticsExport"); + expect(stats.scale).toBe(10); + }); + + test("models the synthetic assumption at the repo's scale", async () => { + const stats = await staticMode(undefined, 1000); + + expect(stats.kind).toBe("fromAssumption"); + expect(stats.scale).toBe(1000); + }); + + test("leaves the mode unscaled when the repo is configured at 1x", async () => { + expect((await staticMode([TABLE], 1)).scale).toBeUndefined(); + }); + + test("leaves the mode unscaled when no scale is configured", async () => { + expect((await staticMode([TABLE])).scale).toBeUndefined(); + }); +}); diff --git a/src/runner.ts b/src/runner.ts index c69d8f2..61283fb 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -14,7 +14,7 @@ import { Connectable } from "./sync/connectable.ts"; import { Remote, StatisticsStrategy } from "./remote/remote.ts"; import { ConnectionManager } from "./sync/connection-manager.ts"; import type { OptimizedQuery } from "./sql/recent-query.ts"; -import { ExportedStats, Statistics } from "@query-doctor/core"; +import { ExportedStats, Statistics, type StatisticsMode } from "@query-doctor/core"; import { readFile } from "node:fs/promises"; import { buildQueries } from "./reporters/site-api.ts"; @@ -37,6 +37,10 @@ export class Runner { // Real production statistics pulled from the Site API. When present, queries // are costed against true prod cardinality instead of synthetic assumptions. productionStats?: ExportedStats[]; + // The repo's configured statistics scale. Queries are then planned against + // this multiple of the data, which answers whether they hold up at a size + // the database hasn't reached yet. + statisticsScale?: number; }) { const remote = options.remote ?? new Remote( options.targetPostgresUrl, @@ -45,7 +49,7 @@ export class Runner { { disableQueryLoader: true } ); await remote.syncFrom(options.sourcePostgresUrl, - await Runner.determineStatsMode(options.statisticsPath, options.productionStats) + await Runner.determineStatsMode(options.statisticsPath, options.productionStats, options.statisticsScale) ); await remote.optimizer.finish; return new Runner( @@ -59,14 +63,23 @@ export class Runner { // Stats-mode precedence for CI: real production stats pulled from the Site API // win, then an explicit stats file, then synthetic assumptions. CI never dumps // stats from the ephemeral target database itself. + // + // `scale` applies to whichever mode wins: core multiplies the planner's view + // of table and index size by it and leaves column statistics alone. A scale of + // 1 is left off the mode entirely, so a repo at the default size posts exactly + // the payload it did before. static async determineStatsMode( statsPath?: string, productionStats?: ExportedStats[], + scale?: number, ): Promise { + const atScale = (stats: T): T => + scale && scale !== 1 ? { ...stats, scale } : stats; + if (productionStats && productionStats.length > 0) { return { type: "static", - stats: Statistics.statsModeFromExport(productionStats), + stats: atScale(Statistics.statsModeFromExport(productionStats)), }; } if (statsPath) { @@ -75,20 +88,20 @@ export class Runner { const stats = ExportedStats.array().parse(rawStats); return { type: "static", - stats: { + stats: atScale({ kind: "fromStatisticsExport", source: { kind: "path", path: statsPath }, stats - } + }) } } return { type: "static", - stats: { + stats: atScale({ kind: "fromAssumption", reltuples: 10_000_000, - } + }) } }