diff --git a/src/remote/remote.ts b/src/remote/remote.ts index f86483f..d72e920 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -25,6 +25,7 @@ import { SchemaLoader } from "./schema-loader.ts"; import { baselineFromDump, detectDrift, + isPastRefreshFloor, type StatsBaseline, } from "./stats-drift.ts"; @@ -92,6 +93,15 @@ export class Remote extends EventEmitter { private statsBaseline?: StatsBaseline; /** Guards against a second drift dump starting while one is in flight. */ private refreshingStats = false; + /** + * When this analyzer last pushed a dump, for the daily floor. A drift- + * triggered push updates it too, so the two triggers can't dump twice in + * quick succession. + */ + private lastStatsPushAt?: number; + /** Earliest time a failed refresh may be retried. See {@link Remote.STATS_RETRY_BACKOFF_MS}. */ + private retryStatsAfter?: number; + private static readonly STATS_RETRY_BACKOFF_MS = 15 * 60 * 1000; private pgStatStatementsStatus: PgStatStatementsStatus = PgStatStatementsStatus.Unknown; @@ -383,26 +393,39 @@ export class Remote extends EventEmitter { * baseline for it would push someone else's numbers as this project's * production statistics. */ - private async refreshStatsIfDrifted(source: Connectable): Promise { + private async refreshStatsIfStale(source: Connectable): Promise { if (!this.statsBaseline || this.refreshingStats) { return; } + // Back off after a failure. `lastStatsPushAt` only advances on success, so + // without this a dump that keeps throwing (a statement_timeout on a large + // pg_statistic read, say) would be retried on every 60s poll. + if (this.retryStatsAfter !== undefined && Date.now() < this.retryStatsAfter) { + return; + } const connector = this.sourceManager.getConnectorFor(source); const reltuples = await connector.getReltuplesByTable(); const verdict = detectDrift(this.statsBaseline, { reltuples }); - if (!verdict.drifted) { + const pastFloor = isPastRefreshFloor(this.lastStatsPushAt, Date.now()); + if (!verdict.drifted && !pastFloor) { return; } this.refreshingStats = true; try { log.info( - `${verdict.kind === "shape" ? "Shape" : "Size"} Drift — ${verdict.reason}. Re-dumping production statistics`, + verdict.drifted + ? `${verdict.kind === "shape" ? "Shape" : "Size"} Drift — ${verdict.reason}. Re-dumping production statistics` + : "Production statistics are past the daily floor. Re-dumping", "remote", ); // applyStatistics records the new baseline and emits `statsApplied`, // which is what carries the dump back to the server. await this.applyStatistics(await this.dumpSourceStats(source)); + this.retryStatsAfter = undefined; + } catch (error) { + this.retryStatsAfter = Date.now() + Remote.STATS_RETRY_BACKOFF_MS; + throw error; } finally { this.refreshingStats = false; } @@ -456,6 +479,13 @@ export class Remote extends EventEmitter { return; } this.statsBaseline = baselineFromDump(stats); + // Arm the daily floor too. The sync path reaches the optimizer directly + // rather than through `applyStatistics`, so without this `lastStatsPushAt` + // stays undefined and the floor never fires for a seeded analyzer — which + // is every analyzer that hasn't happened to drift. Dated from now rather + // than the snapshot's capture time, which the RPC doesn't carry: the floor + // then fires 24h after connect instead of immediately. + this.lastStatsPushAt = Date.now(); } async applyStatistics(statsMode: StatisticsMode): Promise { @@ -467,6 +497,7 @@ export class Remote extends EventEmitter { // database, so drifting against it would be nonsense. if (statsMode.kind === "fromStatisticsExport") { this.statsBaseline = baselineFromDump(stats); + this.lastStatsPushAt = Date.now(); } this.emit("statsApplied", stats); } @@ -518,7 +549,7 @@ export class Remote extends EventEmitter { // The schema poll is also the drift tick: it already runs every 60s, so // checking here costs one `pg_class` read and no extra schedule. this.schemaLoader.on("polled", () => { - this.refreshStatsIfDrifted(source).catch((error) => { + this.refreshStatsIfStale(source).catch((error) => { log.error("Failed to check statistics drift", "remote"); console.error(error); }); diff --git a/src/remote/seed-stats-baseline.test.ts b/src/remote/seed-stats-baseline.test.ts index c73ac2b..2d4ee74 100644 --- a/src/remote/seed-stats-baseline.test.ts +++ b/src/remote/seed-stats-baseline.test.ts @@ -84,3 +84,29 @@ describe("Remote.seedStatsBaseline", () => { expect(pushed).toBe(false); }); }); + +describe("Remote.seedStatsBaseline — daily floor interaction", () => { + function floorArmedAt(remote: Remote): number | undefined { + return (remote as unknown as { lastStatsPushAt?: number }).lastStatsPushAt; + } + + it("arms the daily floor, so a seeded analyzer still refreshes eventually", () => { + // The sync path reaches the optimizer directly rather than through + // applyStatistics, so seeding is the only chance to set this. Left unset, + // isPastRefreshFloor short-circuits on undefined and the floor never fires + // for any analyzer that hasn't happened to drift. + const remote = makeRemote(); + + remote.seedStatsBaseline([table("users", 10_000)]); + + expect(floorArmedAt(remote)).toBeTypeOf("number"); + }); + + it("leaves the floor unarmed when there is no snapshot to seed from", () => { + const remote = makeRemote(); + + remote.seedStatsBaseline([]); + + expect(floorArmedAt(remote)).toBeUndefined(); + }); +}); diff --git a/src/remote/stats-drift.test.ts b/src/remote/stats-drift.test.ts index 392b286..b04aed2 100644 --- a/src/remote/stats-drift.test.ts +++ b/src/remote/stats-drift.test.ts @@ -3,6 +3,7 @@ import type { ExportedStats } from "@query-doctor/core"; import { baselineFromDump, detectDrift, + isPastRefreshFloor, SIZE_DRIFT_MIN_ROWS, } from "./stats-drift.ts"; @@ -149,3 +150,31 @@ describe("detectDrift — Size Drift", () => { expect(verdict.drifted).toBe(false); }); }); + +describe("isPastRefreshFloor", () => { + const NOW = 1_800_000_000_000; + const DAY = 24 * 60 * 60 * 1000; + + it("is false before the analyzer has ever pushed", () => { + // Nothing to compare against, and dumping on a timer for an analyzer with + // no baseline would publish statistics the server never asked for. + expect(isPastRefreshFloor(undefined, NOW)).toBe(false); + }); + + it("is false while the last push is recent", () => { + expect(isPastRefreshFloor(NOW - DAY / 2, NOW)).toBe(false); + }); + + it("is true once a full day has passed", () => { + expect(isPastRefreshFloor(NOW - DAY, NOW)).toBe(true); + }); + + it("is true well past the floor", () => { + expect(isPastRefreshFloor(NOW - DAY * 40, NOW)).toBe(true); + }); + + it("honours a custom floor", () => { + expect(isPastRefreshFloor(NOW - 5_000, NOW, 1_000)).toBe(true); + expect(isPastRefreshFloor(NOW - 500, NOW, 1_000)).toBe(false); + }); +}); diff --git a/src/remote/stats-drift.ts b/src/remote/stats-drift.ts index 2db156a..4b95dd3 100644 --- a/src/remote/stats-drift.ts +++ b/src/remote/stats-drift.ts @@ -143,3 +143,31 @@ function summarize(keys: TableKey[]): string { const rest = keys.length - shown.length; return rest > 0 ? `${shown.join(", ")}, and ${rest} more` : shown.join(", "); } + +/** + * How long a snapshot may stand without a re-dump, regardless of drift. + * + * Shape Drift and Size Drift both watch structure and row counts. Neither sees + * a database whose tables and sizes hold steady while the *distribution* of its + * data moves: histograms, most-common-value lists and correlation all age with + * the values, not the row count. A floor bounds how wrong those can get. + */ +export const DEFAULT_REFRESH_FLOOR_MS = 24 * 60 * 60 * 1000; + +/** + * Whether the last push is old enough to earn a re-dump on its own. + * + * Never true before a first push: with no baseline there is nothing to compare + * against, and dumping on a timer for an analyzer that has never pushed would + * publish statistics for a database the server may not expect. + */ +export function isPastRefreshFloor( + lastPushedAt: number | undefined, + now: number, + floorMs: number = DEFAULT_REFRESH_FLOOR_MS, +): boolean { + if (lastPushedAt === undefined) { + return false; + } + return now - lastPushedAt >= floorMs; +}