diff --git a/.env.example b/.env.example index c643e3c467..ea11129215 100644 --- a/.env.example +++ b/.env.example @@ -1170,19 +1170,6 @@ PROVIDER_LIMITS_SYNC_SPACING_MS=1500 # Used by: src/lib/db/walMaintenance.ts. #OMNIROUTE_WAL_GUARD_MAX_MB=256 -# Minimum rows a cleanup must delete before the post-cleanup VACUUM runs. Default: 1000. -# 0 always vacuums when rows were freed. The post-cleanup VACUUM also runs when the -# reclaimable-space threshold below is met, whichever comes first (either signal fires it). -# Used by: src/lib/db/cleanup.ts::shouldVacuumAfterCleanup(). -#OMNIROUTE_VACUUM_MIN_DELETED_ROWS=1000 - -# Minimum reclaimable space (MB) that alone justifies a full-database VACUUM after a -# cleanup, even when the row-count threshold above was not met (a handful of oversized -# blob rows can free far more space than thousands of tiny rows). VACUUM is synchronous -# and blocks the entire process. Default: 100. 0 always vacuums after any deletion. -# Used by: src/lib/db/cleanup.ts::getVacuumMinReclaimableBytes(). -#OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB=100 - # Explicit path to sql-wasm.wasm for the sql.js fallback adapter. Default: auto-detect. # Used by: src/lib/db/adapters/sqljsAdapter.ts. #OMNIROUTE_SQLJS_WASM_PATH= diff --git a/changelog.d/fixes/12830-cleanup-no-blocking-vacuum.md b/changelog.d/fixes/12830-cleanup-no-blocking-vacuum.md new file mode 100644 index 0000000000..9670f7f8d8 --- /dev/null +++ b/changelog.d/fixes/12830-cleanup-no-blocking-vacuum.md @@ -0,0 +1 @@ +- **fix(db):** the background cleanup scheduler no longer runs a blocking full `VACUUM` after pruning rows (it froze every route, `/healthz` included, for minutes on large databases — 30 s after every start and every 6 h); freed pages are now reclaimed with paced `PRAGMA incremental_vacuum` batches plus a WAL checkpoint, and on `auto_vacuum=NONE` a full VACUUM is deferred to the Storage page's scheduled window via `vacuumScheduler.requestFullVacuum()` ([#12821](https://github.com/diegosouzapw/OmniRoute/issues/12821)) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index d91a92cfbf..a1897e9de7 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -104,8 +104,6 @@ OmniRoute uses **SQLite** (via `better-sqlite3`) for all persistence. These vari | `OMNIROUTE_WAL_TRUNCATE_INTERVAL_MS` | `21600000` (6h) | `src/lib/db/walMaintenance.ts` | Override the periodic `wal_checkpoint(TRUNCATE)` interval (ms). Auto-checkpoint never shrinks the WAL file itself, and a long-running server never closes its DB. `0` disables. | | `OMNIROUTE_WAL_PASSIVE_INTERVAL_MS` | `300000` (5m) | `src/lib/db/walMaintenance.ts` | Override the frequent `wal_checkpoint(PASSIVE)` interval (ms). Keeps pending WAL frames small so the periodic TRUNCATE never copies a multi-GB backlog on the main thread. `0` disables. | | `OMNIROUTE_WAL_GUARD_MAX_MB` | `256` | `src/lib/db/walMaintenance.ts` | When a PASSIVE tick finds the WAL file above this size, escalate to `wal_checkpoint(TRUNCATE)` immediately instead of waiting for the slow tick. | -| `OMNIROUTE_VACUUM_MIN_DELETED_ROWS` | `1000` | `src/lib/db/cleanup.ts` | Post-cleanup VACUUM runs when the cleanup deleted at least this many rows, OR when `OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB` below is met (whichever fires first). `0` means always VACUUM when a cleanup freed any rows; `1` effectively disables the row-count gate. VACUUM rewrites the entire database (multi-GB WAL + I/O burst on large DBs), so tiny cleanups skip it; the Storage page's scheduled VACUUM (default weekly, #4437) and manual VACUUM still reclaim space. | -| `OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB` | `100` | `src/lib/db/cleanup.ts` | Minimum reclaimable space (SQLite's own free-page count, in MB) that alone triggers the post-cleanup `VACUUM`, even when `OMNIROUTE_VACUUM_MIN_DELETED_ROWS` was not met -- a handful of oversized blob rows can free far more space than thousands of tiny rows. `VACUUM` is synchronous and blocks the entire process (15-20 min on a multi-GB database). `0` always vacuums after any deletion. | | `OMNIROUTE_PRESSURE_SELF_RESTART` | `false` | `open-sse/utils/resourcePressure.ts` | Set to `1`/`true`/`yes`/`on` to exit the process after critical resource pressure is sustained for `OMNIROUTE_PRESSURE_SELF_RESTART_AFTER_MS`, letting a supervisor (systemd `Restart=always`, Docker restart policy) bring back a clean process instead of serving 503s indefinitely. | | `OMNIROUTE_PRESSURE_SELF_RESTART_AFTER_MS` | `120000` (2m) | `open-sse/utils/resourcePressure.ts` | How long critical pressure must persist before the self-restart exit fires. | | `OMNIROUTE_SQLJS_WASM_PATH` | _(auto-detect)_ | `src/lib/db/adapters/sqljsAdapter.ts` | Explicit path (absolute or relative to cwd) to `sql-wasm.wasm` when using the `sql.js` WASM fallback adapter. Auto-detected via package dependencies and candidate layouts when unset. | diff --git a/src/lib/db/cleanup.ts b/src/lib/db/cleanup.ts index 10aecd87fc..4cfd87a282 100644 --- a/src/lib/db/cleanup.ts +++ b/src/lib/db/cleanup.ts @@ -4,10 +4,18 @@ * @module lib/db/cleanup */ -import { getDbInstance } from "./core"; -import { getUserDatabaseSettings } from "./databaseSettings"; import { rollupUsageHistoryBeforeDate } from "@/lib/usage/aggregateHistory"; import { purgeCallLogArtifactDirectory } from "@/lib/usage/callLogArtifacts"; + +import { getDbInstance } from "./core"; +import { getUserDatabaseSettings } from "./databaseSettings"; +import { + describeReclaim, + reclaimFreedPages, + type ReclaimFreedPagesOptions, + type ReclaimFreedPagesResult, + type ReclaimStopReason, +} from "./reclaimFreedPages"; import { collectCallLogArtifactsBefore, deleteAllFromTable, @@ -980,131 +988,54 @@ export async function cleanupProxyLogs(): Promise { return result; } +// Post-cleanup space reclamation lives in its own module (#12821, kept out of +// this file to stay under the file-size cap) — re-exported for callers/tests. +export { + reclaimFreedPages, + type ReclaimFreedPagesOptions, + type ReclaimFreedPagesResult, + type ReclaimStopReason, +}; + // ──────────────── Background Cleanup Scheduler ──────────────── const CLEANUP_INTERVAL_MS = 6 * 60 * 60 * 1000; // 6 hours let _cleanupSchedulerTimer: ReturnType | null = null; -const VACUUM_MIN_DELETED_ROWS_DEFAULT = 1000; - -export function getVacuumMinDeletedRows(): number { - const raw = process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS; - if (typeof raw === "string" && raw.trim().length > 0) { - const parsed = Number(raw); - // 0 is valid and means "always VACUUM after a cleanup that freed any rows". - if (Number.isFinite(parsed) && parsed >= 0) return Math.floor(parsed); - } - return VACUUM_MIN_DELETED_ROWS_DEFAULT; -} - /** - * VACUUM rewrites the entire database file (a multi-GB DB produces a - * multi-GB WAL and a matching page-cache/I/O burst on the host). Running it - * after a cleanup that only freed a handful of rows buys no space and pays - * the full rewrite cost, so tiny cleanups skip it; the scheduled VACUUM - * (#4437) and large cleanups still reclaim space. + * One scheduled pass: retention cleanup (`runAutoCleanup` already covers + * proxy_logs), then incremental space reclamation. Exported so tests can drive + * the exact code path the timers run. */ -export function shouldVacuumAfterCleanup( - totalDeleted: number, - minRows: number = getVacuumMinDeletedRows() -): boolean { - return totalDeleted > 0 && totalDeleted >= minRows; -} - -const DEFAULT_VACUUM_MIN_RECLAIMABLE_BYTES = 100 * 1024 * 1024; // 100 MB - -export function getVacuumMinReclaimableBytes(): number { - const raw = process.env.OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB; - if (!raw) return DEFAULT_VACUUM_MIN_RECLAIMABLE_BYTES; - const parsed = Number.parseInt(raw, 10); - return Number.isInteger(parsed) && parsed >= 0 - ? parsed * 1024 * 1024 - : DEFAULT_VACUUM_MIN_RECLAIMABLE_BYTES; -} - -/** - * `db.exec("VACUUM")` is synchronous and blocks the entire process -- on a - * multi-GB database that can freeze all HTTP traffic for 15-20 minutes, even - * when the cleanup that triggered it only freed a handful of rows. Reclaimable - * space (SQLite's own free-page count, not row count) is what actually - * determines whether that multi-minute freeze is worth it: a few oversized - * batch_item_checkpoints rows can free more than thousands of tiny audit-log - * rows. Observed live: a routine cleanup that freed 2-6 rows re-triggered a - * full VACUUM on every restart regardless. - */ -export function getReclaimableBytes(db: ReturnType): number { - const freelist = db.pragma("freelist_count", { simple: true }) as number; - const pageSize = db.pragma("page_size", { simple: true }) as number; - return freelist * pageSize; -} - -/** - * Runs the post-cleanup VACUUM when EITHER the row-count threshold - * (OMNIROUTE_VACUUM_MIN_DELETED_ROWS, default 1000 -- see shouldVacuumAfterCleanup) - * OR the reclaimable-bytes threshold (OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB, default - * 100 MB -- see getReclaimableBytes) is met. The two signals are additive on - * purpose: row count alone misses the case where a handful of oversized blob - * rows (e.g. batch_item_checkpoints) free far more space than thousands of tiny - * audit-log rows would, while reclaimable bytes alone would never fire for a - * cleanup that deletes many small rows without freeing much space yet still - * crosses the operator's row-count comfort threshold. `getReclaimable` is - * optional and defaults to skipping the bytes check entirely, so existing - * callers/tests that only care about the row-count gate keep their exact - * behavior unchanged. Returns true when VACUUM ran. - */ -export async function vacuumAfterCleanup( - totalDeleted: number, - exec: (sql: string) => void, - log: (message: string) => void = (m) => console.log(m), - logError: (message: string, error: unknown) => void = (m, e) => console.error(m, e), - getReclaimable?: () => number -): Promise { - if (totalDeleted <= 0) return false; - const minRows = getVacuumMinDeletedRows(); - const rowThresholdMet = shouldVacuumAfterCleanup(totalDeleted, minRows); - - let reclaimable = 0; - let reclaimableThresholdMet = false; - const minBytes = getVacuumMinReclaimableBytes(); - if (typeof getReclaimable === "function") { - try { - reclaimable = getReclaimable(); - reclaimableThresholdMet = reclaimable >= minBytes; - } catch (err) { - logError("[Cleanup] Failed to read reclaimable space:", err); - } +export async function runScheduledCleanupPass(phase: "startup" | "periodic"): Promise { + const label = phase === "startup" ? "Startup" : "Periodic"; + const result = await runAutoCleanup(); + if (result.totalDeleted > 0) { + console.log(`[Cleanup] ${label} cleanup freed ${result.totalDeleted} rows.`); } - if (!rowThresholdMet && !reclaimableThresholdMet) { - log( - `[Cleanup] Freed ${totalDeleted} rows; skipping VACUUM (below ${minRows}-row threshold` + - (typeof getReclaimable === "function" - ? ` and below ${(minBytes / (1024 * 1024)).toFixed(0)} MB reclaimable, only ${(reclaimable / (1024 * 1024)).toFixed(1)} MB)` - : ")") - ); - return false; - } - log( - `[Cleanup] Running VACUUM (${totalDeleted} rows freed` + - (typeof getReclaimable === "function" - ? `, ${(reclaimable / (1024 * 1024)).toFixed(1)} MB reclaimable)...` - : ")...") - ); + // Always run: it also drains pages left over from a previous capped pass or + // from deletes made outside this scheduler. Costs a few PRAGMA reads when idle. try { - exec("VACUUM"); - log("[Cleanup] VACUUM completed after cleanup."); - return true; - } catch (vacErr) { - logError("[Cleanup] VACUUM after cleanup failed:", vacErr); - return false; + const reclaim = await reclaimFreedPages(); + if (reclaim.stopReason === "error") { + console.error( + `[Cleanup] Space reclamation after ${phase} cleanup stopped early ` + + `(${describeReclaim(reclaim)}): ${reclaim.error}` + ); + } else if (reclaim.mode !== "skipped") { + console.log( + `[Cleanup] Space reclamation after ${phase} cleanup: ${describeReclaim(reclaim)}.` + ); + } + } catch (reclaimErr) { + console.error(`[Cleanup] Space reclamation after ${phase} cleanup failed:`, reclaimErr); } } /** - * Start the background cleanup scheduler. Runs cleanup on startup - * and then every 6 hours. VACUUMs after deletes only when the cleanup freed - * enough rows (OMNIROUTE_VACUUM_MIN_DELETED_ROWS, default 1000) OR freed - * enough reclaimable space (OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB, default - * 100 MB) to justify a full-database rewrite -- see vacuumAfterCleanup(). + * Start the background cleanup scheduler. Runs cleanup on startup and then + * every 6 hours, then reclaims freed pages incrementally (never a blocking + * full VACUUM — see the reclamation section above and #12821). * * Without the cleanup itself, tables grow unboundedly (compression_analytics * 600K+ rows, usage_history 250K+ rows) causing 1.4GB+ SQLite files and @@ -1116,19 +1047,7 @@ export function startCleanupScheduler(): void { // Run cleanup 30s after startup (let the server initialize first). setTimeout(async () => { try { - const result = await runAutoCleanup(); - const proxyResult = await cleanupProxyLogs(); - const totalDeleted = result.totalDeleted + proxyResult.deleted; - if (totalDeleted > 0) { - console.log(`[Cleanup] Startup cleanup freed ${totalDeleted} rows.`); - await vacuumAfterCleanup( - totalDeleted, - (sql) => getDbInstance().exec(sql), - undefined, - undefined, - () => getReclaimableBytes(getDbInstance()) - ); - } + await runScheduledCleanupPass("startup"); } catch (err) { console.error("[Cleanup] Startup cleanup failed:", err); } @@ -1137,19 +1056,7 @@ export function startCleanupScheduler(): void { // Schedule periodic cleanup every 6 hours. _cleanupSchedulerTimer = setInterval(async () => { try { - const result = await runAutoCleanup(); - const proxyResult = await cleanupProxyLogs(); - const totalDeleted = result.totalDeleted + proxyResult.deleted; - if (totalDeleted > 0) { - console.log(`[Cleanup] Periodic cleanup freed ${totalDeleted} rows.`); - await vacuumAfterCleanup( - totalDeleted, - (sql) => getDbInstance().exec(sql), - undefined, - undefined, - () => getReclaimableBytes(getDbInstance()) - ); - } + await runScheduledCleanupPass("periodic"); } catch (err) { console.error("[Cleanup] Periodic cleanup failed:", err); } diff --git a/src/lib/db/reclaimFreedPages.ts b/src/lib/db/reclaimFreedPages.ts new file mode 100644 index 0000000000..037eb19ed2 --- /dev/null +++ b/src/lib/db/reclaimFreedPages.ts @@ -0,0 +1,258 @@ +/** + * Post-cleanup space reclamation (#12821). + * + * Never run a full `VACUUM` from the cleanup scheduler. `node:sqlite` is + * synchronous, so a full rebuild of a multi-hundred-MB file holds the event + * loop for minutes — no route answers, `/healthz` included, and in-flight + * streams stall. Full rebuilds belong to `vacuumScheduler`, which runs them in + * the window the operator configured on the Storage page + * (`scheduledVacuum` / `vacuumHour`). + * + * What we do instead depends on the database's `auto_vacuum` mode: + * + * - INCREMENTAL: freed pages sit on SQLite's freelist and + * `PRAGMA incremental_vacuum(N)` hands back at most N of them per call. We + * drain the list in ~1 MiB batches and pause between batches for about as + * long as the last batch took, so the event loop stays at least half free + * for requests. Whatever is left after the per-pass caps is picked up by the + * next pass. + * - FULL: SQLite already returns freed pages on commit; nothing to do. + * - NONE: `incremental_vacuum` is a no-op, so only a full rebuild can shrink + * the file. We record the request with `vacuumScheduler` and let it run in + * its configured window (or never, if the operator said so). + * + * In WAL mode the truncation only reaches the main file at a checkpoint, so a + * pass folds the WAL back periodically and ends with `wal_checkpoint(TRUNCATE)`; + * otherwise the `.sqlite` would keep its size and the `-wal` would sit at its + * high-water mark until the next 6-hourly checkpoint. + * + * @module lib/db/reclaimFreedPages + */ + +import type { SqliteAdapter } from "./adapters/types"; +import { getDbInstance } from "./core"; +import { requestFullVacuum } from "./vacuumScheduler"; + +/** Target bytes moved per `incremental_vacuum` call; converted to pages via `page_size`. */ +const INCREMENTAL_VACUUM_BATCH_BYTES = 1024 * 1024; +/** Hard cap per cleanup pass (≈2 GiB at the target batch size); the rest waits for the next pass. */ +const INCREMENTAL_VACUUM_MAX_BATCHES = 2048; +/** Wall-clock budget per cleanup pass, so a slow disk cannot turn a pass into a long tail. */ +const INCREMENTAL_VACUUM_TIME_BUDGET_MS = 30_000; +/** Longest pause between batches; a batch pauses for as long as it took, capped here. */ +const INCREMENTAL_VACUUM_MAX_PAUSE_MS = 250; +/** PASSIVE checkpoint cadence within a pass so the WAL is folded back as we go. */ +const INCREMENTAL_VACUUM_CHECKPOINT_EVERY_BATCHES = 64; + +// `PRAGMA auto_vacuum` values (https://sqlite.org/pragma.html#pragma_auto_vacuum): 0 = NONE. +const AUTO_VACUUM_FULL = 1; +const AUTO_VACUUM_INCREMENTAL = 2; + +export type ReclaimStopReason = + /** The freelist reached zero. */ + | "drained" + /** The per-pass batch cap was hit; the remainder waits for the next pass. */ + | "max-batches" + /** The per-pass wall-clock budget was hit; the remainder waits for the next pass. */ + | "time-budget" + /** Another connection held the write lock (SQLITE_BUSY/LOCKED); retry next pass. */ + | "busy" + /** The DB handle was closed under us (backup restore / import); retry next pass. */ + | "closed" + /** A batch threw something other than BUSY; see `error`. Earlier batches are committed. */ + | "error"; + +export interface ReclaimFreedPagesResult { + /** + * - `incremental`: freed pages were reclaimed via bounded `incremental_vacuum` batches. + * - `auto`: `auto_vacuum = FULL` — SQLite reclaims on commit, nothing to do here. + * - `deferred`: `auto_vacuum = NONE` — a full VACUUM was requested from `vacuumScheduler`. + * - `skipped`: the freelist was already empty. + */ + mode: "incremental" | "auto" | "deferred" | "skipped"; + autoVacuum: number; + pageSize: number; + freelistBefore: number; + freelistAfter: number; + batches: number; + durationMs: number; + /** Only set for `incremental`. */ + stopReason?: ReclaimStopReason; + /** Only set when `stopReason === "error"`. */ + error?: string; +} + +export interface ReclaimFreedPagesOptions { + /** @internal test seam — overrides the `page_size`-derived batch. */ + batchPages?: number; + /** @internal test seam */ + maxBatches?: number; + /** @internal test seam */ + timeBudgetMs?: number; + /** @internal test seam — pause between batches; receives the last batch's duration in ms. */ + pause?: (lastBatchMs: number) => Promise; + /** @internal test seam */ + now?: () => number; +} + +function readPragmaNumber(db: SqliteAdapter, pragma: string): number { + const value = db.pragma(pragma, { simple: true }); + const numeric = typeof value === "number" ? value : Number(value); + return Number.isFinite(numeric) ? numeric : 0; +} + +function pauseBetweenBatches(lastBatchMs: number): Promise { + const delay = Math.min(INCREMENTAL_VACUUM_MAX_PAUSE_MS, Math.max(0, lastBatchMs)); + return new Promise((resolve) => setTimeout(resolve, delay)); +} + +function isBusyError(err: unknown): boolean { + const details = err as { code?: unknown; errcode?: unknown; message?: unknown } | null; + const code = details?.code ?? details?.errcode; + // better-sqlite3 / bun:sqlite expose the symbolic code; node:sqlite exposes the numeric one. + if (typeof code === "string" && /^SQLITE_(BUSY|LOCKED)/.test(code)) return true; + if (code === 5 || code === 6) return true; // SQLITE_BUSY / SQLITE_LOCKED + const message = err instanceof Error ? err.message : String(details?.message ?? err); + return /database is locked|SQLITE_BUSY|SQLITE_LOCKED/i.test(message); +} + +function checkpointQuietly(db: SqliteAdapter, mode: "PASSIVE" | "TRUNCATE"): void { + if (!db.open) return; + try { + db.checkpoint(mode); + } catch { + // Best effort: the 6-hourly TRUNCATE checkpoint in core.ts will catch up. + } +} + +/** + * Reclaim pages freed by cleanup without blocking the event loop. + * + * Safe to call after every cleanup pass regardless of how many rows were + * deleted: when the freelist is empty this is a handful of cheap PRAGMA reads. + * Never throws for per-batch failures — the result carries `stopReason` (and + * `error`) so callers can log partial progress. Exported for tests and for + * callers that free space outside the scheduler. + */ +export async function reclaimFreedPages( + options: ReclaimFreedPagesOptions = {} +): Promise { + const maxBatches = Math.max(1, Math.floor(options.maxBatches ?? INCREMENTAL_VACUUM_MAX_BATCHES)); + const timeBudgetMs = options.timeBudgetMs ?? INCREMENTAL_VACUUM_TIME_BUDGET_MS; + const pause = options.pause ?? pauseBetweenBatches; + const now = options.now ?? Date.now; + + const db = getDbInstance(); + const startedAt = now(); + const autoVacuum = readPragmaNumber(db, "auto_vacuum"); + const pageSize = readPragmaNumber(db, "page_size") || 4096; + const freelistBefore = readPragmaNumber(db, "freelist_count"); + const batchPages = Math.max( + 1, + Math.floor(options.batchPages ?? INCREMENTAL_VACUUM_BATCH_BYTES / pageSize) + ); + + const finish = ( + mode: ReclaimFreedPagesResult["mode"], + freelistAfter: number, + batches: number, + stopReason?: ReclaimStopReason, + error?: string + ): ReclaimFreedPagesResult => ({ + mode, + autoVacuum, + pageSize, + freelistBefore, + freelistAfter, + batches, + durationMs: now() - startedAt, + ...(stopReason ? { stopReason } : {}), + ...(error ? { error } : {}), + }); + + if (freelistBefore <= 0) return finish("skipped", freelistBefore, 0); + + if (autoVacuum === AUTO_VACUUM_FULL) return finish("auto", freelistBefore, 0); + + if (autoVacuum !== AUTO_VACUUM_INCREMENTAL) { + // NONE (or an unknown value): incremental reclamation is impossible. + // Hand the decision to the scheduler; never rebuild here. + requestFullVacuum( + `cleanup left ${freelistBefore} free page(s) that auto_vacuum=${autoVacuum} cannot reclaim incrementally` + ); + return finish("deferred", freelistBefore, 0); + } + + let freelist = freelistBefore; + let batches = 0; + let lastBatchMs = 0; + let stopReason: ReclaimStopReason = "drained"; + let error: string | undefined; + + while (freelist > 0) { + if (batches >= maxBatches) { + stopReason = "max-batches"; + break; + } + if (batches > 0) await pause(lastBatchMs); + // The handle can be closed while we were paused (backup restore, DB import). + if (!db.open) { + stopReason = "closed"; + break; + } + + const batchStartedAt = now(); + try { + // `exec`, not `pragma()`: incremental_vacuum is a stepping pragma that frees one + // page per step, and bun:sqlite's `all()` stops after the first zero-column row. + db.exec(`PRAGMA incremental_vacuum(${batchPages})`); + } catch (err) { + if (isBusyError(err)) { + stopReason = "busy"; + } else { + stopReason = "error"; + error = err instanceof Error ? err.message : String(err); + } + break; + } + lastBatchMs = now() - batchStartedAt; + batches += 1; + freelist = readPragmaNumber(db, "freelist_count"); + + if (batches % INCREMENTAL_VACUUM_CHECKPOINT_EVERY_BATCHES === 0) { + checkpointQuietly(db, "PASSIVE"); + } + if (freelist > 0 && now() - startedAt >= timeBudgetMs) { + stopReason = "time-budget"; + break; + } + } + + // In WAL mode the file only shrinks when the truncating commit is checkpointed. + if (batches > 0) checkpointQuietly(db, "TRUNCATE"); + + return finish("incremental", freelist, batches, stopReason, error); +} + +export function describeReclaim(result: ReclaimFreedPagesResult): string { + switch (result.mode) { + case "skipped": + return "freelist already empty"; + case "auto": + return `auto_vacuum=FULL reclaims on commit (${result.freelistBefore} page(s) pending)`; + case "deferred": + return `auto_vacuum=NONE — ${result.freelistBefore} free page(s) left for the scheduled VACUUM`; + case "incremental": { + const reclaimed = result.freelistBefore - result.freelistAfter; + const mib = ((reclaimed * result.pageSize) / (1024 * 1024)).toFixed(1); + const tail = + result.freelistAfter > 0 + ? `, ${result.freelistAfter} left for the next pass (${result.stopReason})` + : ""; + return ( + `reclaimed ${reclaimed} page(s) (~${mib} MiB) in ${result.batches} batch(es) ` + + `over ${result.durationMs}ms${tail}` + ); + } + } +} diff --git a/src/lib/db/vacuumScheduler.ts b/src/lib/db/vacuumScheduler.ts index 143313075b..191c558985 100644 --- a/src/lib/db/vacuumScheduler.ts +++ b/src/lib/db/vacuumScheduler.ts @@ -52,6 +52,15 @@ export interface VacuumSchedulerState { lastDurationMs: number | null; isRunning: boolean; nextRunAt: number | null; + /** + * Set when another subsystem decided a full VACUUM is warranted but deferred + * it to this scheduler's configured window instead of running it inline + * (e.g. `cleanup.ts` on an `auto_vacuum = NONE` database, where + * `incremental_vacuum` cannot reclaim anything — see #12821). Cleared by the + * next successful run. Persisted so the request survives restarts. + */ + fullVacuumRequestedAt: number | null; + fullVacuumRequestReason: string | null; /** #13432 — configured vs live auto_vacuum mismatch pending reconcile, or null once reconciled. */ autoVacuumDrift: AutoVacuumDrift | null; /** Pages freed by the most recent bounded `PRAGMA incremental_vacuum` batch, or null if the last run was a full VACUUM / drift reconcile. */ @@ -83,6 +92,8 @@ const STATE_DEFAULTS: VacuumSchedulerState = { lastDurationMs: null, isRunning: false, nextRunAt: null, + fullVacuumRequestedAt: null, + fullVacuumRequestReason: null, autoVacuumDrift: null, lastReclaimedPages: null, }; @@ -97,6 +108,7 @@ const AUTO_VACUUM_DRIFT_KEY = "vacuumDrift"; const INCREMENTAL_VACUUM_BATCH_PAGES = 2000; let timer: ReturnType | null = null; +let hydrated = false; let currentState: VacuumSchedulerState = { ...STATE_DEFAULTS }; function isRecord(value: unknown): value is Record { @@ -343,6 +355,9 @@ export async function runNow(): Promise<{ currentState.lastReclaimedPages = reclaimedPages; currentState.autoVacuumDrift = loadAutoVacuumDrift(); currentState.isRunning = false; + // A full rebuild just happened — any deferred request is satisfied. + currentState.fullVacuumRequestedAt = null; + currentState.fullVacuumRequestReason = null; refresh(); // reset the next-run clock from this successful run return { success: true, durationMs: duration, reclaimedPages: reclaimedPages ?? undefined }; } catch (err) { @@ -357,6 +372,58 @@ export async function runNow(): Promise<{ } } +/** + * Merge the persisted blob into `currentState` once per process. `init()` does + * this too; having it here means an early `requestFullVacuum()` (before + * `init()`, e.g. when init failed non-fatally) cannot overwrite a persisted + * `lastRunAt` with the in-memory default and pull the next run forward. + */ +function hydrateFromPersistedState(): void { + if (hydrated) return; + hydrated = true; + const persisted = loadPersistedState(); + currentState = { + ...STATE_DEFAULTS, + ...persisted, + isRunning: false, // never resume a "running" state across restarts + nextRunAt: null, // recomputed by refresh() + // Always reload from the drift record's own key_value entry rather than + // trusting a stale copy embedded in the scheduler state blob — it is the + // source of truth optimizationSettings.ts writes at every boot. + autoVacuumDrift: loadAutoVacuumDrift(), + }; +} + +/** + * Record that a full VACUUM is warranted without running it now (#12821). + * + * Contract: the first request's timestamp is kept (so the UI can show how long + * it has been pending), the reason is overwritten with the latest one, and the + * request is cleared by the next successful `runNow()` — scheduled or manual. + * `scheduledVacuum = never` is honored: the request stays visible in + * `getState()`, nothing runs automatically. + */ +export function requestFullVacuum(reason: string): VacuumSchedulerState { + hydrateFromPersistedState(); + const firstRequest = currentState.fullVacuumRequestedAt === null; + if (firstRequest) currentState.fullVacuumRequestedAt = Date.now(); + currentState.fullVacuumRequestReason = reason; + persistState(); + + if (firstRequest) { + let when: string; + if (readScheduleSettings().scheduledVacuum === "never") { + when = "scheduledVacuum is 'never' — run it manually from the Storage page when convenient"; + } else if (currentState.nextRunAt !== null) { + when = `deferred to the scheduled run at ${new Date(currentState.nextRunAt).toISOString()}`; + } else { + when = "deferred to the next scheduled run"; + } + console.log(`[VacuumScheduler] Full VACUUM requested (${reason}); ${when}.`); + } + return getState(); +} + /** * Initialize the scheduler. Called once from the Next.js * `instrumentation-node.ts` register() hook. Safe to call multiple @@ -365,17 +432,8 @@ export async function runNow(): Promise<{ export function init(): VacuumSchedulerState { if (timer) return getState(); - const persisted = loadPersistedState(); - currentState = { - ...STATE_DEFAULTS, - ...persisted, - isRunning: false, // never resume a "running" state across restarts - nextRunAt: null, // recompute below - // Always reload from the drift record's own key_value entry rather than - // trusting a stale copy embedded in the scheduler state blob — it is the - // source of truth optimizationSettings.ts writes at every boot. - autoVacuumDrift: loadAutoVacuumDrift(), - }; + hydrated = false; // an explicit init() always re-reads the persisted blob + hydrateFromPersistedState(); return refresh(); } @@ -401,5 +459,6 @@ export function stop(): void { */ export function __resetForTests(): void { stop(); + hydrated = false; currentState = { ...STATE_DEFAULTS }; } diff --git a/tests/unit/cleanup-column-fix.test.mjs b/tests/unit/cleanup-column-fix.test.mjs index 060743021a..60369068ed 100644 --- a/tests/unit/cleanup-column-fix.test.mjs +++ b/tests/unit/cleanup-column-fix.test.mjs @@ -7,7 +7,11 @@ import path from "node:path"; // These verify the critical column name fixes that were causing silent cleanup failures. const CLEANUP_PATH = path.resolve(import.meta.dirname, "../../src/lib/db/cleanup.ts"); +const RECLAIM_PATH = path.resolve(import.meta.dirname, "../../src/lib/db/reclaimFreedPages.ts"); const source = fs.readFileSync(CLEANUP_PATH, "utf-8"); +// Post-cleanup space reclamation (#12821) lives in its own module, kept out of +// cleanup.ts to stay under the file-size cap — scan both for the invariants below. +const reclaimSource = fs.readFileSync(RECLAIM_PATH, "utf-8"); test("cleanup: compression_analytics uses 'timestamp' column (not 'created_at')", () => { // The bug: cleanup used WHERE created_at < ? but the table has 'timestamp' column. @@ -57,13 +61,16 @@ test("cleanup: has background scheduler (startCleanupScheduler)", () => { source.includes("startCleanupScheduler"), "must export startCleanupScheduler for periodic background cleanup" ); + assert.ok(source.includes("CLEANUP_INTERVAL_MS"), "must have a cleanup interval constant"); + // #12821: reclaim freed pages without a blocking full VACUUM on the serving thread. assert.ok( - source.includes("CLEANUP_INTERVAL_MS"), - "must have a cleanup interval constant" + reclaimSource.includes("incremental_vacuum("), + "reclaimFreedPages() must reclaim freed pages via PRAGMA incremental_vacuum after deletes" ); assert.ok( - source.includes("VACUUM"), - "scheduler must run VACUUM after deletes to reclaim disk space" + !/\b(exec|run|prepare)\s*\(\s*[`'"]\s*VACUUM\b/i.test(source) && + !/\b(exec|run|prepare)\s*\(\s*[`'"]\s*VACUUM\b/i.test(reclaimSource), + "scheduler must never run a blocking full VACUUM — defer to vacuumScheduler (#12821)" ); }); @@ -114,10 +121,7 @@ test("cleanup: a2a_task_events uses correct table name (not 'a2a_events')", () = }); test("cleanup: memories uses correct table name (not 'memory_entries')", () => { - assert.ok( - source.includes("DELETE FROM memories WHERE"), - "must use correct table name memories" - ); + assert.ok(source.includes("DELETE FROM memories WHERE"), "must use correct table name memories"); assert.ok( !source.includes("DELETE FROM memory_entries WHERE"), "must NOT use non-existent table name memory_entries" diff --git a/tests/unit/db-cleanup-vacuum-gate.test.ts b/tests/unit/db-cleanup-vacuum-gate.test.ts deleted file mode 100644 index 81f6c9e798..0000000000 --- a/tests/unit/db-cleanup-vacuum-gate.test.ts +++ /dev/null @@ -1,101 +0,0 @@ -/** - * Post-cleanup VACUUM gate. - * - * VACUUM rewrites the entire database file: on a ~3GB DB that is a 3GB WAL plus a - * full page-cache/I/O burst on the host. The cleanup scheduler used to run it after - * every non-empty cleanup — including startup cleanups that freed ~100 rows — which - * is pure churn. The gate skips VACUUM unless the cleanup freed enough rows to - * justify a full rewrite (OMNIROUTE_VACUUM_MIN_DELETED_ROWS, default 1000). - */ -import assert from "node:assert/strict"; -import { describe, it } from "node:test"; -import { - getVacuumMinDeletedRows, - shouldVacuumAfterCleanup, - vacuumAfterCleanup, -} from "../../src/lib/db/cleanup.ts"; - -describe("shouldVacuumAfterCleanup", () => { - it("skips VACUUM when nothing was deleted", () => { - assert.equal(shouldVacuumAfterCleanup(0, 1000), false); - }); - - it("skips VACUUM for cleanups below the threshold", () => { - assert.equal(shouldVacuumAfterCleanup(1, 1000), false); - assert.equal(shouldVacuumAfterCleanup(999, 1000), false); - }); - - it("runs VACUUM at and above the threshold", () => { - assert.equal(shouldVacuumAfterCleanup(1000, 1000), true); - assert.equal(shouldVacuumAfterCleanup(5000, 1000), true); - }); - - it("treats a 0 threshold as always-vacuum", () => { - const saved = process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS; - process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS = "0"; - try { - assert.equal(getVacuumMinDeletedRows(), 0); - assert.equal(shouldVacuumAfterCleanup(1, 0), true); - } finally { - if (saved === undefined) delete process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS; - else process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS = saved; - } - }); - - it("defaults to 1000 rows and honors the env override", () => { - const saved = process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS; - try { - delete process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS; - assert.equal(getVacuumMinDeletedRows(), 1000); - process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS = "50"; - assert.equal(getVacuumMinDeletedRows(), 50); - assert.equal(shouldVacuumAfterCleanup(60), true); - process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS = "not-a-number"; - assert.equal(getVacuumMinDeletedRows(), 1000); - } finally { - if (saved === undefined) delete process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS; - else process.env.OMNIROUTE_VACUUM_MIN_DELETED_ROWS = saved; - } - }); -}); - -describe("vacuumAfterCleanup", () => { - it("does not exec VACUUM below the threshold but says why", async () => { - const execed: string[] = []; - const logs: string[] = []; - const ran = await vacuumAfterCleanup( - 125, - (sql) => execed.push(sql), - (m) => logs.push(m) - ); - assert.equal(ran, false); - assert.deepEqual(execed, []); - assert.ok(logs.some((line) => line.includes("skipping VACUUM"))); - }); - - it("execs VACUUM once when enough rows were freed", async () => { - const execed: string[] = []; - const ran = await vacuumAfterCleanup( - 1000, - (sql) => execed.push(sql), - () => {}, - () => {} - ); - assert.equal(ran, true); - assert.deepEqual(execed, ["VACUUM"]); - }); - - it("swallows VACUUM failures into an error log like the old inline path", async () => { - const errLogs: string[] = []; - const ran = await vacuumAfterCleanup( - 5000, - () => { - throw new Error("disk full"); - }, - () => {}, - (m) => errLogs.push(m) - ); - assert.equal(ran, false); - assert.ok(errLogs.some((line) => line.includes("VACUUM after cleanup failed"))); - }); -}); diff --git a/tests/unit/db-wal-passive-scheduler.test.ts b/tests/unit/db-wal-passive-scheduler.test.ts index bac4d1c7ab..29089a33c9 100644 --- a/tests/unit/db-wal-passive-scheduler.test.ts +++ b/tests/unit/db-wal-passive-scheduler.test.ts @@ -83,6 +83,5 @@ test("the new env vars are documented", () => { const docs = readSource("docs/reference/ENVIRONMENT.md"); assert.match(docs, /OMNIROUTE_WAL_PASSIVE_INTERVAL_MS/); assert.match(docs, /OMNIROUTE_WAL_GUARD_MAX_MB/); - assert.match(docs, /OMNIROUTE_VACUUM_MIN_DELETED_ROWS/); assert.match(docs, /OMNIROUTE_PRESSURE_SELF_RESTART/); }); diff --git a/tests/unit/db/cleanup-reclaim-freed-pages.test.ts b/tests/unit/db/cleanup-reclaim-freed-pages.test.ts new file mode 100644 index 0000000000..1da61966ec --- /dev/null +++ b/tests/unit/db/cleanup-reclaim-freed-pages.test.ts @@ -0,0 +1,291 @@ +/** + * Regression tests for #12821 — the cleanup scheduler must never run a + * blocking full `VACUUM` on the serving thread. + * + * Covers `reclaimFreedPages()` and `runScheduledCleanupPass()` in + * src/lib/db/cleanup.ts: + * 1. INCREMENTAL: freed pages are reclaimed in bounded `incremental_vacuum` + * batches, pausing between batches, and the WAL is checkpointed so the + * main file actually shrinks. + * 2. Batch / time caps stop a pass early and leave the rest for the next one. + * 3. FULL: nothing to do — SQLite reclaims on commit, freelist is empty. + * 4. NONE: no rebuild happens here; a full VACUUM is requested from + * `vacuumScheduler` instead and the file keeps its page count. + * 5. Empty freelist → `skipped` without touching the scheduler. + * 6. The scheduled pass itself (the code the timers run) defers on NONE. + * + * The source-level "no VACUUM statement in cleanup.ts" invariant lives in + * tests/unit/cleanup-column-fix.test.mjs alongside the other cleanup.ts scans. + * + * DB isolation mirrors tests/unit/db/vacuum-scheduler.test.ts: temp DATA_DIR, + * resetDbInstance() before the suite, cleanup in test.after(). + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cleanup-reclaim-")); +const originalDataDir = process.env.DATA_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../../src/lib/db/core.ts"); +core.resetDbInstance(); + +const cleanup = await import("../../../src/lib/db/cleanup.ts"); +const scheduler = await import("../../../src/lib/db/vacuumScheduler.ts"); + +type AutoVacuumMode = 0 | 1 | 2; + +const noPause = async () => {}; + +function pragmaNumber(pragma: string): number { + return Number(core.getDbInstance().pragma(pragma, { simple: true })); +} + +/** + * `auto_vacuum` only changes on a rebuilt file, and VACUUM cannot run in WAL + * mode while the journal is shared — so switch to DELETE journaling for the + * rebuild the same way optimizationSettings.ts does, then back to WAL. + */ +function setAutoVacuum(mode: AutoVacuumMode): void { + const db = core.getDbInstance(); + if (pragmaNumber("auto_vacuum") === mode) return; + db.pragma("journal_mode = DELETE"); + db.pragma(`auto_vacuum = ${mode}`); + db.exec("VACUUM"); + db.pragma("journal_mode = WAL"); + assert.equal(pragmaNumber("auto_vacuum"), mode, `auto_vacuum should now be ${mode}`); +} + +/** Insert then delete ~`rows` KiB of blobs so the freelist has something on it. */ +function churnPages(rows = 1500): void { + const db = core.getDbInstance(); + db.exec("CREATE TABLE IF NOT EXISTS reclaim_churn (id INTEGER PRIMARY KEY, payload BLOB)"); + const insert = db.prepare("INSERT INTO reclaim_churn (payload) VALUES (?)"); + const blob = Buffer.alloc(1024, 0xab); + const fill = db.transaction(() => { + for (let i = 0; i < rows; i += 1) insert.run(blob); + }); + fill(); + db.prepare("DELETE FROM reclaim_churn").run(); +} + +test.beforeEach(() => { + scheduler.__resetForTests(); + const db = core.getDbInstance(); + db.prepare("DELETE FROM key_value WHERE namespace IN ('scheduler', 'databaseSettings')").run(); + db.exec("DROP TABLE IF EXISTS reclaim_churn"); +}); + +test.after(() => { + scheduler.__resetForTests(); + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + if (originalDataDir === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = originalDataDir; +}); + +test("INCREMENTAL: freed pages are reclaimed in batches, pausing between them", async () => { + setAutoVacuum(2); + churnPages(); + const before = pragmaNumber("freelist_count"); + const pageCountBefore = pragmaNumber("page_count"); + assert.ok(before > 8, `expected a populated freelist, got ${before}`); + + const pauses: number[] = []; + const result = await cleanup.reclaimFreedPages({ + batchPages: Math.max(1, Math.floor(before / 8)), + pause: async (lastBatchMs) => { + pauses.push(lastBatchMs); + }, + }); + + assert.equal(result.mode, "incremental"); + assert.equal(result.stopReason, "drained"); + assert.equal(result.autoVacuum, 2); + assert.equal(result.pageSize, pragmaNumber("page_size")); + assert.equal(result.freelistBefore, before); + assert.equal(result.freelistAfter, 0, "freelist should be fully drained"); + assert.equal(pragmaNumber("freelist_count"), 0); + assert.ok(result.batches > 1, `expected several batches, got ${result.batches}`); + assert.equal(pauses.length, result.batches - 1, "must pause between every pair of batches"); + assert.ok( + pauses.every((ms) => Number.isFinite(ms) && ms >= 0), + "pause receives the last batch's duration" + ); + // The pass ends with a TRUNCATE checkpoint, so the truncation reached the main file. + assert.ok( + pragmaNumber("page_count") < pageCountBefore, + `page_count should shrink (${pageCountBefore} -> ${pragmaNumber("page_count")})` + ); + assert.equal(scheduler.getState().fullVacuumRequestedAt, null, "no full VACUUM request"); +}); + +test("INCREMENTAL: default batch size is derived from page_size (~1 MiB)", async () => { + setAutoVacuum(2); + churnPages(4000); // ~4 MiB of blobs → a few default-size batches + const before = pragmaNumber("freelist_count"); + const pageSize = pragmaNumber("page_size"); + const expectedBatchPages = Math.max(1, Math.floor((1024 * 1024) / pageSize)); + + const result = await cleanup.reclaimFreedPages({ pause: noPause }); + + assert.equal(result.mode, "incremental"); + assert.equal(result.freelistAfter, 0); + assert.equal( + result.batches, + Math.ceil(before / expectedBatchPages), + `expected ceil(${before} / ${expectedBatchPages}) batches` + ); +}); + +test("INCREMENTAL: maxBatches caps a pass and leaves the remainder for the next one", async () => { + setAutoVacuum(2); + churnPages(); + const before = pragmaNumber("freelist_count"); + + const result = await cleanup.reclaimFreedPages({ + batchPages: 1, + maxBatches: 3, + pause: noPause, + }); + + assert.equal(result.mode, "incremental"); + assert.equal(result.stopReason, "max-batches"); + assert.equal(result.batches, 3); + // incremental_vacuum(1) frees *up to* one page per step (a pointer-map page can absorb a step). + const after = pragmaNumber("freelist_count"); + assert.ok( + after >= before - 3 && after < before, + `expected ${before}-3..${before}-1, got ${after}` + ); + assert.equal(result.freelistAfter, after); +}); + +test("INCREMENTAL: the time budget stops a pass early", async () => { + setAutoVacuum(2); + churnPages(); + const before = pragmaNumber("freelist_count"); + + // Fake clock: every now() call advances 6ms, so the elapsed time crosses a + // 10ms budget after the first batch and the loop must stop with pages left. + let tick = 0; + const result = await cleanup.reclaimFreedPages({ + batchPages: 1, + timeBudgetMs: 10, + now: () => { + tick += 6; + return tick; + }, + pause: noPause, + }); + + assert.equal(result.mode, "incremental"); + assert.equal(result.stopReason, "time-budget"); + assert.ok( + result.batches >= 1 && result.batches < before, + `stopped early after ${result.batches} of ${before}` + ); + assert.ok(result.freelistAfter > 0, "some pages must remain for the next pass"); +}); + +test("FULL: nothing to reclaim by hand — SQLite already did it on commit", async () => { + setAutoVacuum(1); + churnPages(); + // auto_vacuum=FULL truncates on the DELETE's commit, so the freelist is empty + // and the pass is a no-op that never runs an incremental batch. + assert.equal(pragmaNumber("freelist_count"), 0); + + const result = await cleanup.reclaimFreedPages({ pause: noPause }); + + assert.equal(result.autoVacuum, 1); + assert.equal(result.mode, "skipped"); + assert.equal(result.batches, 0); + assert.equal(scheduler.getState().fullVacuumRequestedAt, null); +}); + +test("NONE: no rebuild here — a full VACUUM is requested from vacuumScheduler instead", async () => { + setAutoVacuum(0); + churnPages(); + const before = pragmaNumber("freelist_count"); + const pageCountBefore = pragmaNumber("page_count"); + assert.ok(before > 0, `expected free pages with auto_vacuum=NONE, got ${before}`); + + const result = await cleanup.reclaimFreedPages({ pause: noPause }); + + assert.equal(result.mode, "deferred"); + assert.equal(result.autoVacuum, 0); + assert.equal(result.batches, 0); + assert.equal(result.freelistAfter, before, "free pages must be untouched (no VACUUM ran)"); + assert.equal(pragmaNumber("freelist_count"), before); + // A VACUUM would have rebuilt the file and dropped page_count; it must be unchanged. + assert.equal(pragmaNumber("page_count"), pageCountBefore, "the file must not have been rebuilt"); + + const state = scheduler.getState(); + assert.ok(typeof state.fullVacuumRequestedAt === "number", "request must be recorded"); + assert.match(state.fullVacuumRequestReason ?? "", /auto_vacuum=0/); + assert.match(state.fullVacuumRequestReason ?? "", new RegExp(`${before} free page`)); +}); + +test("empty freelist → skipped, scheduler untouched", async () => { + setAutoVacuum(2); + await cleanup.reclaimFreedPages({ pause: noPause }); // drain leftovers from earlier tests + assert.equal(pragmaNumber("freelist_count"), 0); + + const result = await cleanup.reclaimFreedPages({ pause: noPause }); + + assert.equal(result.mode, "skipped"); + assert.equal(result.batches, 0); + assert.equal(result.freelistBefore, 0); + assert.equal(result.stopReason, undefined); + assert.equal(scheduler.getState().fullVacuumRequestedAt, null); +}); + +test("runScheduledCleanupPass(): the timers' code path defers on NONE instead of rebuilding", async () => { + setAutoVacuum(0); + churnPages(); + const before = pragmaNumber("freelist_count"); + const pageCountBefore = pragmaNumber("page_count"); + assert.ok(before > 0); + + const logged: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { + logged.push(args.map(String).join(" ")); + }; + try { + await cleanup.runScheduledCleanupPass("startup"); + } finally { + console.log = originalLog; + } + + // Retention cleanup runs before reclamation, and one of its targets + // (cleanupCompressionRunTelemetry) lazily creates its table on first use — + // a real, one-time, unrelated page cost from a freshly migrated DB, not + // something reclaimFreedPages() does. Tolerate that noise while still + // catching the regression this test guards against: incremental_vacuum (or + // a rebuild) draining most/all of the freelist, which the assertion below + // on `page_count` also independently rules out. + const freelistAfter = pragmaNumber("freelist_count"); + assert.ok( + freelistAfter >= before - 5, + `expected no meaningful reclamation on NONE (freelist ~${before}), got ${freelistAfter}` + ); + assert.equal(pragmaNumber("page_count"), pageCountBefore, "no rebuild on NONE"); + assert.ok( + typeof scheduler.getState().fullVacuumRequestedAt === "number", + "the pass must hand the full VACUUM to vacuumScheduler" + ); + assert.ok( + logged.some((line) => + /\[Cleanup\] Space reclamation after startup cleanup: auto_vacuum=NONE/.test(line) + ), + `expected the reclamation summary line, got:\n${logged.join("\n")}` + ); + assert.ok( + !logged.some((line) => /Running VACUUM/.test(line)), + "the old blocking-VACUUM log line must be gone" + ); +}); diff --git a/tests/unit/db/vacuum-scheduler.test.ts b/tests/unit/db/vacuum-scheduler.test.ts index 43fb6e9bb0..6e6aac94b6 100644 --- a/tests/unit/db/vacuum-scheduler.test.ts +++ b/tests/unit/db/vacuum-scheduler.test.ts @@ -202,3 +202,75 @@ test("lastRunAt survives a simulated restart (state reloaded from key_value)", a assert.equal(afterRestart, beforeRestart); scheduler.stop(); }); + +// ──────────────── #12821: deferred full-VACUUM requests ──────────────── + +test("getState() exposes the deferred full-VACUUM request fields as null by default", () => { + const state = scheduler.getState(); + assert.equal(state.fullVacuumRequestedAt, null); + assert.equal(state.fullVacuumRequestReason, null); +}); + +test("requestFullVacuum() records the request, persists it, and keeps the first timestamp", () => { + setOptimizationSettings({ scheduledVacuum: "weekly", vacuumHour: 2 }); + scheduler.init(); + try { + const first = scheduler.requestFullVacuum("cleanup left 42 free page(s)"); + assert.equal(typeof first.fullVacuumRequestedAt, "number"); + assert.equal(first.fullVacuumRequestReason, "cleanup left 42 free page(s)"); + // It must NOT run the VACUUM itself — only record intent. + assert.equal(first.lastRunAt, null); + assert.equal(first.isRunning, false); + + const second = scheduler.requestFullVacuum("cleanup left 7 free page(s)"); + assert.equal(second.fullVacuumRequestedAt, first.fullVacuumRequestedAt, "first timestamp wins"); + assert.equal( + second.fullVacuumRequestReason, + "cleanup left 7 free page(s)", + "latest reason wins" + ); + + // Persisted in the same key_value blob as the rest of the state. + const row = core + .getDbInstance() + .prepare("SELECT value FROM key_value WHERE namespace = 'scheduler' AND key = 'vacuum'") + .get() as { value: string } | undefined; + assert.ok(row, "state row must exist"); + const persisted = JSON.parse(row.value); + assert.equal(persisted.fullVacuumRequestedAt, first.fullVacuumRequestedAt); + assert.equal(persisted.fullVacuumRequestReason, "cleanup left 7 free page(s)"); + } finally { + scheduler.stop(); + } +}); + +test("a deferred request survives a simulated restart and is cleared by the next successful run", async () => { + setOptimizationSettings({ scheduledVacuum: "never" }); + scheduler.init(); + scheduler.requestFullVacuum("auto_vacuum=0 cannot reclaim incrementally"); + const requestedAt = scheduler.getState().fullVacuumRequestedAt; + assert.equal(typeof requestedAt, "number"); + + // Restart: in-memory state wiped, init() reloads from key_value. + scheduler.__resetForTests(); + assert.equal(scheduler.getState().fullVacuumRequestedAt, null); + scheduler.init(); + assert.equal(scheduler.getState().fullVacuumRequestedAt, requestedAt); + assert.match(scheduler.getState().fullVacuumRequestReason ?? "", /auto_vacuum=0/); + + // scheduledVacuum=never is honored: nothing is armed even though a request is pending. + assert.equal(scheduler.getState().enabled, false); + assert.equal(scheduler.getState().nextRunAt, null); + + // The next successful full run (scheduled or manual via the Storage page) satisfies it. + try { + const result = await scheduler.runNow(); + assert.equal(result.success, true); + const state = scheduler.getState(); + assert.equal(state.fullVacuumRequestedAt, null); + assert.equal(state.fullVacuumRequestReason, null); + assert.notEqual(state.lastRunAt, null); + } finally { + scheduler.stop(); + } +}); diff --git a/tests/unit/vacuum-reclaimable-threshold.test.ts b/tests/unit/vacuum-reclaimable-threshold.test.ts deleted file mode 100644 index 0dbd7707f9..0000000000 --- a/tests/unit/vacuum-reclaimable-threshold.test.ts +++ /dev/null @@ -1,132 +0,0 @@ -// The auto-cleanup scheduler used to run a full `VACUUM` after ANY deletion -// (`totalDeleted > 0`), no matter how small. `db.exec("VACUUM")` is synchronous -// and blocks the entire process -- on a multi-GB database that freezes all HTTP -// traffic for 15-20 minutes. Observed live: a routine cleanup that freed 2-6 -// rows re-triggered a full VACUUM on every restart, because the new terminal- -// batch cleanup (see db-terminal-batch-and-file-cleanup.test.ts) almost always -// finds a handful of newly-aged-out batches. -// -// Row count was never the right signal anyway: a few oversized -// batch_item_checkpoints rows can free far more space than thousands of tiny -// audit-log rows. This pins vacuumAfterCleanup()'s reclaimable-bytes gate: SQLite's -// own free-page count (PRAGMA freelist_count), not "were any rows deleted". The -// row-count gate (OMNIROUTE_VACUUM_MIN_DELETED_ROWS) is untouched and still fires -// on its own -- see db-cleanup-vacuum-gate.test.ts -- this file only pins the -// ADDITIONAL reclaimable-bytes trigger. - -import test from "node:test"; -import assert from "node:assert/strict"; -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; - -const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-vacuum-threshold-")); -process.env.DATA_DIR = TEST_DATA_DIR; - -const core = await import("../../src/lib/db/core.ts"); -const cleanup = await import("../../src/lib/db/cleanup.ts"); - -test.after(() => { - core.resetDbInstance(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); -}); - -async function withEnv( - key: string, - value: string | undefined, - fn: () => T | Promise -): Promise { - const prev = process.env[key]; - if (value === undefined) delete process.env[key]; - else process.env[key] = value; - try { - return await fn(); - } finally { - if (prev === undefined) delete process.env[key]; - else process.env[key] = prev; - } -} - -test("getVacuumMinReclaimableBytes: defaults to 100 MB", async () => { - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", undefined, () => { - assert.equal(cleanup.getVacuumMinReclaimableBytes(), 100 * 1024 * 1024); - }); -}); - -test("getVacuumMinReclaimableBytes: honors the env override, including 0 (always vacuum)", async () => { - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", "5", () => { - assert.equal(cleanup.getVacuumMinReclaimableBytes(), 5 * 1024 * 1024); - }); - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", "0", () => { - assert.equal(cleanup.getVacuumMinReclaimableBytes(), 0); - }); -}); - -test("getVacuumMinReclaimableBytes: rejects garbage and falls back to the default", async () => { - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", "not-a-number", () => { - assert.equal(cleanup.getVacuumMinReclaimableBytes(), 100 * 1024 * 1024); - }); - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", "-1", () => { - assert.equal(cleanup.getVacuumMinReclaimableBytes(), 100 * 1024 * 1024); - }); -}); - -test("vacuumAfterCleanup: reclaimable-bytes gate skips VACUUM when space is under the threshold (row gate also below threshold)", async () => { - const db = core.getDbInstance(); - db.exec("CREATE TABLE IF NOT EXISTS vacuum_threshold_probe (id INTEGER PRIMARY KEY, v TEXT)"); - // A handful of tiny rows leaves negligible freelist space after deletion -- - // nowhere near the (very high, deliberately unreachable in this test) threshold. - db.exec("INSERT INTO vacuum_threshold_probe (v) VALUES ('a'), ('b'), ('c')"); - db.exec("DELETE FROM vacuum_threshold_probe"); - - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", "999999", async () => { - // Also keep the row-count gate from firing on its own so only the - // reclaimable-bytes gate is under test here. - await withEnv("OMNIROUTE_VACUUM_MIN_DELETED_ROWS", "999999", async () => { - // Must not throw, and specifically must not attempt to run VACUUM at all -- - // proven by the fact that a VACUUM would otherwise reset freelist_count. - const before = cleanup.getReclaimableBytes(db); - const ran = await cleanup.vacuumAfterCleanup( - 3, - (sql) => db.exec(sql), - () => {}, - () => {}, - () => cleanup.getReclaimableBytes(db) - ); - const after = cleanup.getReclaimableBytes(db); - assert.equal(ran, false); - assert.equal(after, before, "skipping VACUUM must leave the freelist untouched"); - }); - }); -}); - -test("vacuumAfterCleanup: reclaimable-bytes gate alone triggers VACUUM even when the row-count gate is not met", async () => { - const db = core.getDbInstance(); - db.exec("CREATE TABLE IF NOT EXISTS vacuum_threshold_probe2 (id INTEGER PRIMARY KEY, v TEXT)"); - const insert = db.prepare("INSERT INTO vacuum_threshold_probe2 (v) VALUES (?)"); - const big = "x".repeat(4096); - for (let i = 0; i < 200; i++) insert.run(big); - db.exec("DELETE FROM vacuum_threshold_probe2"); - - const reclaimableBeforeVacuum = cleanup.getReclaimableBytes(db); - assert.ok(reclaimableBeforeVacuum > 0, "sanity: the delete above must have freed some pages"); - - await withEnv("OMNIROUTE_VACUUM_MIN_RECLAIMABLE_MB", "0", async () => { - // Row-count gate deliberately unreachable: only 1 row "deleted" here, far - // below any realistic OMNIROUTE_VACUUM_MIN_DELETED_ROWS value, proving the - // reclaimable-bytes signal alone is sufficient to trigger VACUUM. - await withEnv("OMNIROUTE_VACUUM_MIN_DELETED_ROWS", "999999", async () => { - const ran = await cleanup.vacuumAfterCleanup( - 1, - (sql) => db.exec(sql), - () => {}, - () => {}, - () => cleanup.getReclaimableBytes(db) - ); - assert.equal(ran, true); - }); - }); - - // A successful VACUUM rebuilds the file with no free pages left over. - assert.equal(cleanup.getReclaimableBytes(db), 0, "VACUUM must have actually run"); -});