perf(selfhost): tune Postgres autovacuum and document the observation-write batching decision

src/github/backfill.ts

@@ -3541,6 +3541,20 @@ function summarizeSegments(
   };
 }
 
+// #2543: this is the ONLY call site of recordGitHubRateLimitObservation -- one row per outbound GitHub REST/
+// GraphQL response. DELIBERATELY left un-batched (documented decision, not an oversight): the write rate is
+// bounded by GitHub's own REST budget for a single App installation (~5000/hour ≈ 1.4/s sustained, further
+// capped in practice by QUEUE_CONCURRENCY's small worker-pool size), nowhere near a volume where single-row
+// Postgres INSERTs meaningfully pressure the connection pool. shouldWaitForGitHubRateLimit (rate-limit.ts)
+// reads the LATEST row from this exact table for admission control across every self-host queue worker
+// (including in a multi-instance/shared-Postgres deployment, where a buffering instance would make its own
+// writes stale to every OTHER instance's reads, not just its own) -- a batching window here trades a real,
+// bounded-scale write-volume concern for a genuine risk to the admission-control freshness the #1936 rate-
+// limit-reliability campaign was built around: a stale "remaining: 500" observation would let a queue worker
+// admit a job it should have deferred, right when conserving the budget matters most. Revisit ONLY if this
+// table's write volume is ever independently measured to actually pressure the pool -- the table-level
+// autovacuum tuning (tuneGithubRateLimitObservationsAutovacuum, src/selfhost/pg-adapter.ts) already addresses
+// the dead-tuple-bloat half of this issue, which is the part that was actually observable/anticipated.
 async function recordGitHubResponse(
   env: Env,
   repoFullName: string | null,

src/selfhost/pg-adapter.ts

@@ -83,3 +83,34 @@ export function createPgAdapter(pool: Pool): D1Database {
   };
   return adapter as unknown as D1Database;
 }
+
+// #2543: github_rate_limit_observations receives one INSERT per outbound GitHub API response and is pruned in
+// daily bulk deletes by the retention job (pruneExpiredRecords) -- an insert-then-bulk-delete pattern that is
+// exactly the shape that causes dead-tuple bloat under Postgres's stock autovacuum settings (scale_factor 0.2,
+// i.e. autovacuum waits for 20% of the table to be dead before vacuuming -- fine for a slowly-growing table,
+// too lax for one that gets emptied in one daily burst). Lowering the scale factor makes autovacuum reclaim
+// space promptly after each day's bulk delete instead of letting dead tuples accumulate across cycles. A
+// storage-parameter ALTER is idempotent (re-applying the same value is a no-op), so this runs unconditionally
+// on every Postgres boot rather than needing its own migration-ledger tracking. SQLite has no autovacuum
+// concept at all, so this must never run there -- callers gate it behind the Postgres backend check, matching
+// PGPOOL_MAX/resolvePostgresPoolMax's own "server.ts wiring, tested logic elsewhere" split (src/selfhost/
+// queue-common.ts), since server.ts itself has no test harness (top-level main(), Codecov-ignored).
+export const GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL =
+  "ALTER TABLE github_rate_limit_observations SET (autovacuum_vacuum_scale_factor = 0.05, autovacuum_vacuum_threshold = 50)";
+
+/** Apply the autovacuum tuning above via the SAME D1Database.exec() surface runSelfHostMigrations already uses
+ *  for migrations -- so this reuses translateDdl's existing SQL path rather than a second raw-pool query
+ *  mechanism. Must be called AFTER migrations (the table has to exist first); best-effort by design (a
+ *  storage-parameter tweak is an optimization, never a correctness dependency -- a failure here must not stop
+ *  the self-host from booting). */
+export async function tuneGithubRateLimitObservationsAutovacuum(db: D1Database): Promise<void> {
+  await db.exec(GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL).catch((error: unknown) => {
+    console.error(
+      JSON.stringify({
+        level: "warn",
+        event: "selfhost_autovacuum_tune_failed",
+        error: error instanceof Error ? error.message : String(error),
+      }),
+    );
+  });
+}

src/server.ts

@@ -50,7 +50,7 @@ import {
 } from "./selfhost/health";
 import { gauge, incr, observe, renderMetrics } from "./selfhost/metrics";
 import { runSelfHostMigrations } from "./selfhost/migrate";
-import { createPgAdapter } from "./selfhost/pg-adapter";
+import { createPgAdapter, tuneGithubRateLimitObservationsAutovacuum } from "./selfhost/pg-adapter";
 import { createPgQueue } from "./selfhost/pg-queue";
 import { createPgVectorize, initPgVectorize } from "./selfhost/pg-vectorize";
 import { resolvePostgresPoolMax } from "./selfhost/queue-common";
@@ -370,6 +370,9 @@ async function main(): Promise<void> {
   console.log(
     JSON.stringify({ event: "selfhost_migrations_applied", count: applied }),
   );
+  // #2543: Postgres-only, applied AFTER migrations (the table must already exist). No-op on SQLite, which has
+  // no autovacuum concept at all -- gated on the same usePostgres check the backend was built from.
+  if (usePostgres) await tuneGithubRateLimitObservationsAutovacuum(backend.db);
 
   const ai = createSelfHostAi(process.env);
   if (ai)

test/integration/selfhost-pg.test.ts

@@ -5,7 +5,7 @@
 import { afterAll, beforeAll, describe, expect, it } from "vitest";
 import pg from "pg";
 import { runSelfHostMigrations } from "../../src/selfhost/migrate";
-import { createPgAdapter } from "../../src/selfhost/pg-adapter";
+import { createPgAdapter, tuneGithubRateLimitObservationsAutovacuum } from "../../src/selfhost/pg-adapter";
 import { pruneExpiredRecords } from "../../src/db/retention";
 import { processJob } from "../../src/queue/processors";
 
@@ -84,4 +84,18 @@ suite("Postgres backend (#977) — real Postgres", () => {
     const audit = await db.prepare("SELECT outcome FROM audit_events WHERE event_type = ?").bind("retention.prune").first<{ outcome: string }>();
     expect(audit?.outcome).toBe("success");
   });
+
+  it("tunes github_rate_limit_observations autovacuum below Postgres's default, idempotently (#2543)", async () => {
+    const db = createPgAdapter(pool);
+
+    await tuneGithubRateLimitObservationsAutovacuum(db);
+    await tuneGithubRateLimitObservationsAutovacuum(db); // idempotent -- a second apply must not throw
+
+    const row = await pool.query<{ reloptions: string[] | null }>(
+      "SELECT reloptions FROM pg_class WHERE relname = 'github_rate_limit_observations'",
+    );
+    const options = row.rows[0]?.reloptions ?? [];
+    expect(options).toContain("autovacuum_vacuum_scale_factor=0.05");
+    expect(options).toContain("autovacuum_vacuum_threshold=50");
+  });
 });

test/unit/selfhost-pg-adapter-autovacuum.test.ts

@@ -0,0 +1,76 @@
+// Unit tests for the github_rate_limit_observations autovacuum tuning step (#2543). Uses a mock D1Database
+// (just the .exec() surface runSelfHostMigrations already relies on) so no real Postgres is required -- the
+// SQL itself is plain, already-Postgres-native syntax with no SQLite constructs for pg-dialect.ts to translate,
+// so a mocked interaction test is a faithful, fast substitute for a live ALTER TABLE.
+import { describe, expect, it, vi } from "vitest";
+import {
+  GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL,
+  tuneGithubRateLimitObservationsAutovacuum,
+} from "../../src/selfhost/pg-adapter";
+
+function mockDb(execImpl: (sql: string) => Promise<unknown>): D1Database {
+  return { exec: vi.fn(execImpl) } as unknown as D1Database;
+}
+
+describe("GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL (#2543)", () => {
+  it("targets the github_rate_limit_observations table with a scale factor below Postgres's 0.2 default", () => {
+    expect(GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL).toContain("github_rate_limit_observations");
+    expect(GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL).toContain("autovacuum_vacuum_scale_factor");
+    const match = GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL.match(/autovacuum_vacuum_scale_factor\s*=\s*([\d.]+)/);
+    expect(match).not.toBeNull();
+    expect(Number(match?.[1])).toBeLessThan(0.2);
+    expect(Number(match?.[1])).toBeGreaterThan(0);
+  });
+
+  it("is a single idempotent storage-parameter ALTER, not an additive/destructive DDL statement", () => {
+    expect(GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL.trim().toUpperCase()).toMatch(/^ALTER TABLE/);
+    expect(GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL).not.toMatch(/DROP|DELETE|TRUNCATE/i);
+  });
+});
+
+describe("tuneGithubRateLimitObservationsAutovacuum (#2543)", () => {
+  it("applies the autovacuum SQL via db.exec()", async () => {
+    const db = mockDb(async () => ({ count: 1, duration: 0 }));
+
+    await tuneGithubRateLimitObservationsAutovacuum(db);
+
+    expect(db.exec).toHaveBeenCalledWith(GITHUB_RATE_LIMIT_OBSERVATIONS_AUTOVACUUM_SQL);
+    expect(db.exec).toHaveBeenCalledTimes(1);
+  });
+
+  it("fails open (does not throw) when db.exec rejects -- an optimization, never a boot-blocking dependency", async () => {
+    const db = mockDb(async () => {
+      throw new Error("connection reset");
+    });
+    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+    await expect(tuneGithubRateLimitObservationsAutovacuum(db)).resolves.toBeUndefined();
+
+    expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("selfhost_autovacuum_tune_failed"));
+    errorSpy.mockRestore();
+  });
+
+  it("logs the underlying error message on failure", async () => {
+    const db = mockDb(async () => {
+      throw new Error("relation does not exist");
+    });
+    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+    await tuneGithubRateLimitObservationsAutovacuum(db);
+
+    expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("relation does not exist"));
+    errorSpy.mockRestore();
+  });
+
+  it("stringifies a non-Error rejection instead of throwing on error.message access", async () => {
+    const db = mockDb(async () => {
+      throw "a plain string rejection";
+    });
+    const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+    await expect(tuneGithubRateLimitObservationsAutovacuum(db)).resolves.toBeUndefined();
+
+    expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("a plain string rejection"));
+    errorSpy.mockRestore();
+  });
+});