Files
OmniRoute/tests/unit/cleanup-column-fix.test.mjs
Innokentiy Solntsev e7872e57c3 fix(db): reclaim freed pages incrementally instead of a blocking VACUUM in the cleanup scheduler (#12821) (#12830)
* fix(db): reclaim freed pages incrementally instead of a blocking VACUUM in the cleanup scheduler (#12821)

startCleanupScheduler() ran a synchronous whole-database VACUUM on the event
loop whenever a cleanup pass deleted at least one row - 30 s after every
start and every 6 h. With node:sqlite that blocks every route (/healthz
included) for the duration: 7 min 55 s on a 540 MB storage.sqlite to reclaim
six rows. It also bypassed vacuumScheduler, the app-level owner of full
VACUUMs and the operator's scheduledVacuum / vacuumHour settings.

cleanup.ts no longer issues a full VACUUM. After each pass reclaimFreedPages()
branches on PRAGMA auto_vacuum:

- INCREMENTAL: drain the freelist with PRAGMA incremental_vacuum(N) in ~1 MiB
  batches (N from page_size), pausing between batches for as long as the last
  one took (<=250 ms), PASSIVE checkpoint every 64 batches and a TRUNCATE
  checkpoint at the end so the main file shrinks in WAL mode; hard caps of
  2048 batches / 30 s per pass, the remainder waits for the next pass.
- FULL: nothing to do, SQLite reclaims on commit.
- NONE: incremental_vacuum is a no-op, so record a request via the new
  vacuumScheduler.requestFullVacuum(); the rebuild runs in the configured
  window (or via the Storage page button). scheduledVacuum=never is honored.

vacuumScheduler persists fullVacuumRequestedAt / fullVacuumRequestReason,
clears them on the next successful runNow(), and hydrates from key_value
before an early request so it cannot clobber a persisted lastRunAt.

Loop robustness: db.exec() rather than pragma() (bun:sqlite's all() steps a
zero-column pragma once), SQLITE_BUSY/LOCKED and a handle closed under the
pass stop it quietly, other errors stop it with partial progress logged.
Also drops the duplicate cleanupProxyLogs() call in the scheduled pass -
runAutoCleanup() already covers proxy_logs.

Tests: new tests/unit/db/cleanup-reclaim-freed-pages.test.ts (INCREMENTAL
drain/pause/checkpoint, page_size-derived batch, caps, FULL no-op, NONE
defers and leaves page_count untouched, runScheduledCleanupPass() path);
vacuum-scheduler.test.ts covers requestFullVacuum persistence, restart
survival and clearing; cleanup-column-fix.test.mjs now asserts
incremental_vacuum and the absence of a full VACUUM statement.

* chore(changelog): name the #12821 fragment after its PR (#12830)

* fix(db): extract reclaimFreedPages into its own module and fix full-suite regressions

Split the #12821 incremental-vacuum reclamation logic out of cleanup.ts
into src/lib/db/reclaimFreedPages.ts (re-exported for callers/tests) so
cleanup.ts stays under the file-size cap after the #13011 reconciliation
merge grew it past the 1200-line threshold.

Also fixes two full-suite failures surfaced by running the
cleanup/vacuumScheduler/db-health suite post-merge (not just this PR's
own 3 test files, per the plan-file's mandatory item):

- tests/unit/cleanup-column-fix.test.mjs scanned cleanup.ts's raw source
  for the PRAGMA incremental_vacuum invariant, which now lives in the
  extracted module — updated to scan both files.
- tests/unit/db/cleanup-reclaim-freed-pages.test.ts asserted the freelist
  count is byte-for-byte unchanged when auto_vacuum=NONE. The tip's
  runAutoCleanup() now also runs cleanupCompressionRunTelemetry(), which
  lazily creates its table on first use (ensureCompressionRunTelemetryTable)
  — a legitimate one-time page cost from a freshly migrated DB, unrelated
  to reclaimFreedPages()'s own behavior. Loosened the assertion to a small
  tolerance while keeping the page_count assertion that actually guards
  against a full rebuild.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* chore(db): drop the reclaimable-bytes VACUUM gate test superseded by incremental reclaim

tests/unit/vacuum-reclaimable-threshold.test.ts pinned cleanup.ts's
vacuumAfterCleanup()/getReclaimableBytes()/getVacuumMinReclaimableBytes()
(#13079). This branch removes the inline post-cleanup full VACUUM entirely in
favour of reclaimFreedPages() (#12821), which reads the same freelist_count /
page_size signal and defers a full VACUUM to the vacuum scheduler when
auto_vacuum=NONE. With those three exports gone the file cannot compile, and
the behaviour it guarded no longer exists.

---------

Co-authored-by: insoln <is@careerum.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
2026-09-17 18:38:26 -03:00

156 lines
5.9 KiB
JavaScript

import test from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import path from "node:path";
// Source-level invariant tests for cleanup.ts fixes.
// 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.
// This caused silent failures — 600K+ rows accumulated over 52 days.
assert.ok(
source.includes("DELETE FROM compression_analytics WHERE timestamp < ?"),
"compression_analytics cleanup must use 'timestamp' column, not 'created_at'"
);
assert.ok(
!source.includes("DELETE FROM compression_analytics WHERE created_at"),
"must NOT use created_at for compression_analytics (column doesn't exist)"
);
});
test("cleanup: call_logs uses 'timestamp' column (not 'created_at')", () => {
// Same bug as compression_analytics.
assert.ok(
source.includes("DELETE FROM call_logs WHERE timestamp < ?"),
"call_logs cleanup must use 'timestamp' column"
);
assert.ok(
!source.includes("DELETE FROM call_logs WHERE created_at"),
"must NOT use created_at for call_logs (column doesn't exist)"
);
});
test("cleanup: has proxy_logs cleanup function", () => {
assert.ok(
source.includes("cleanupProxyLogs"),
"must have cleanupProxyLogs function for the proxy_logs table"
);
assert.ok(
source.includes("DELETE FROM proxy_logs WHERE timestamp < ?"),
"proxy_logs cleanup must use timestamp column"
);
});
test("cleanup: proxy_logs is included in runAutoCleanup", () => {
assert.ok(
source.includes("proxyLogs: await cleanupProxyLogs()"),
"runAutoCleanup must include proxyLogs cleanup"
);
});
test("cleanup: has background scheduler (startCleanupScheduler)", () => {
assert.ok(
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(
reclaimSource.includes("incremental_vacuum("),
"reclaimFreedPages() must reclaim freed pages via PRAGMA incremental_vacuum after deletes"
);
assert.ok(
!/\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)"
);
});
test("cleanup: scheduler is wired into instrumentation-node.ts", () => {
const instrumentationPath = path.resolve(
import.meta.dirname,
"../../src/instrumentation-node.ts"
);
const instrumentation = fs.readFileSync(instrumentationPath, "utf-8");
assert.ok(
instrumentation.includes("startCleanupScheduler"),
"instrumentation-node.ts must import startCleanupScheduler"
);
assert.ok(
instrumentation.includes("startCleanupScheduler()"),
"instrumentation-node.ts must call startCleanupScheduler() at startup"
);
});
test("cleanup: mcp_tool_audit uses correct table name (not 'mcp_audit_log')", () => {
assert.ok(
source.includes("DELETE FROM mcp_tool_audit WHERE created_at < ?"),
"mcp_tool_audit cleanup must use its created_at column"
);
assert.ok(
!source.includes("DELETE FROM mcp_audit_log WHERE"),
"must NOT use non-existent table name mcp_audit_log"
);
assert.ok(
!source.includes("DELETE FROM mcp_tool_audit WHERE timestamp"),
"must NOT use timestamp for mcp_tool_audit"
);
});
test("cleanup: a2a_task_events uses correct table name (not 'a2a_events')", () => {
assert.ok(
source.includes("DELETE FROM a2a_task_events WHERE created_at < ?"),
"a2a_task_events cleanup must use its created_at column"
);
assert.ok(
!source.includes("DELETE FROM a2a_events WHERE"),
"must NOT use non-existent table name a2a_events"
);
assert.ok(
!source.includes("DELETE FROM a2a_task_events WHERE timestamp"),
"must NOT use timestamp for a2a_task_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 memory_entries WHERE"),
"must NOT use non-existent table name memory_entries"
);
});
test("cleanup: mcp_tool_audit prunes by created_at (existing column), not timestamp", () => {
// mcp_tool_audit has created_at (see 002_mcp_a2a_tables.sql); timestamp does
// not exist, so WHERE timestamp < ? raised SqliteError "no such column" at
// every boot-time cleanup and the retention pruning never ran.
assert.ok(
source.includes("DELETE FROM mcp_tool_audit WHERE created_at < ?"),
"mcp_tool_audit cleanup must use created_at column"
);
assert.ok(
!source.includes("DELETE FROM mcp_tool_audit WHERE timestamp"),
"must NOT use timestamp for mcp_tool_audit (column doesn't exist)"
);
});
test("cleanup: a2a_task_events prunes by created_at (existing column), not timestamp", () => {
// Same schema fact for a2a_task_events (created_at, no timestamp column).
assert.ok(
source.includes("DELETE FROM a2a_task_events WHERE created_at < ?"),
"a2a_task_events cleanup must use created_at column"
);
assert.ok(
!source.includes("DELETE FROM a2a_task_events WHERE timestamp"),
"must NOT use timestamp for a2a_task_events (column doesn't exist)"
);
});