mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-16 20:02:45 +03:00
Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).
This commit is contained in:
committed by
GitHub
parent
842c32f6f0
commit
d2fadb01bc
1
changelog.d/fixes/13432-incremental-auto-vacuum-drift.md
Normal file
1
changelog.d/fixes/13432-incremental-auto-vacuum-drift.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(db):** reconcile `auto_vacuum` drift between the configured INCREMENTAL mode and the live SQLite pragma — detected at startup and reconciled out-of-request by the vacuum scheduler, which now also runs a bounded `PRAGMA incremental_vacuum` reclaim instead of an unconditional full `VACUUM` once INCREMENTAL is actually in effect (#13432) — thanks @tolgaaksoy
|
||||
@@ -260,6 +260,8 @@ export function getDatabaseSettings(): DatabaseSettings {
|
||||
vacuumState.lastRunAt !== null ? new Date(vacuumState.lastRunAt).toISOString() : null,
|
||||
lastOptimizationAt: null,
|
||||
integrityCheck: getIntegrityCheck(),
|
||||
autoVacuumDrift: vacuumState.autoVacuumDrift,
|
||||
lastReclaimedPages: vacuumState.lastReclaimedPages,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -4,7 +4,29 @@ import type { SqliteAdapter } from "./adapters/types";
|
||||
|
||||
type SqliteDatabase = SqliteAdapter;
|
||||
type DatabaseOptimizationSettings = DatabaseSettings["optimization"];
|
||||
type AutoVacuumMode = DatabaseOptimizationSettings["autoVacuumMode"];
|
||||
export type AutoVacuumMode = DatabaseOptimizationSettings["autoVacuumMode"];
|
||||
|
||||
/**
|
||||
* A mismatch between the configured `optimization.autoVacuumMode` (the
|
||||
* `key_value` config store) and the live SQLite `auto_vacuum` pragma on the
|
||||
* actual database file. See #13432 — migration 046 seeds the config value on
|
||||
* every database (including pre-existing ones) but SQLite only applies
|
||||
* `auto_vacuum` on a subsequent `VACUUM`, which the startup path deliberately
|
||||
* never runs synchronously (that would reintroduce the blocking-VACUUM
|
||||
* hazard tracked by #12821).
|
||||
*/
|
||||
export interface AutoVacuumDrift {
|
||||
configured: AutoVacuumMode;
|
||||
live: AutoVacuumMode;
|
||||
}
|
||||
|
||||
// Shared key_value coordinate for the persisted drift record. Written here
|
||||
// (at boot, directly against the `db` handle being initialized — NOT via
|
||||
// getDbInstance(), which is not yet set at this point in core.ts's boot
|
||||
// sequence) and read/cleared by vacuumScheduler.ts once the scheduler runs
|
||||
// the reconcile out-of-request.
|
||||
const AUTO_VACUUM_DRIFT_NAMESPACE = "scheduler";
|
||||
const AUTO_VACUUM_DRIFT_KEY = "vacuumDrift";
|
||||
|
||||
const AUTO_VACUUM_MODE_TO_PRAGMA: Record<AutoVacuumMode, number> = {
|
||||
NONE: 0,
|
||||
@@ -233,6 +255,40 @@ export function applyDatabaseOptimizationSettingsForDb(
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Compares the configured `autoVacuumMode` against the live SQLite pragma.
|
||||
* Pure/read-only — never mutates the database. Returns `null` when they
|
||||
* already agree.
|
||||
*/
|
||||
export function checkAutoVacuumDrift(
|
||||
db: SqliteDatabase,
|
||||
settings: DatabaseOptimizationSettings
|
||||
): AutoVacuumDrift | null {
|
||||
const liveMode = getAutoVacuumModeForDb(db);
|
||||
if (liveMode === settings.autoVacuumMode) return null;
|
||||
return { configured: settings.autoVacuumMode, live: liveMode };
|
||||
}
|
||||
|
||||
/**
|
||||
* Persists (or clears, when `drift` is `null`) the auto_vacuum drift record
|
||||
* directly against the given `db` handle. Deliberately does NOT go through
|
||||
* `getDbInstance()` — at the one call site that matters (startup, inside
|
||||
* core.ts before `setDb()` has run) that would recurse back into database
|
||||
* initialization.
|
||||
*/
|
||||
function persistAutoVacuumDriftRecord(db: SqliteDatabase, drift: AutoVacuumDrift | null): void {
|
||||
try {
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
AUTO_VACUUM_DRIFT_NAMESPACE,
|
||||
AUTO_VACUUM_DRIFT_KEY,
|
||||
JSON.stringify(drift)
|
||||
);
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
console.warn(`[DB] Failed to persist auto_vacuum drift record: ${message}`);
|
||||
}
|
||||
}
|
||||
|
||||
export function applyStoredDatabaseOptimizationSettings(db: SqliteDatabase): void {
|
||||
const settings = readDatabaseOptimizationSettings(db);
|
||||
// Startup can happen concurrently in test workers and clustered hosts. Only
|
||||
@@ -241,6 +297,19 @@ export function applyStoredDatabaseOptimizationSettings(db: SqliteDatabase): voi
|
||||
applyDatabaseOptimizationSettingsForDb(db, settings, {
|
||||
applyPersistent: false,
|
||||
});
|
||||
|
||||
// #13432: detect (but never synchronously fix — that would reintroduce the
|
||||
// #12821 blocking-VACUUM-at-boot hazard) a drift between the configured
|
||||
// autoVacuumMode and the live pragma. Reconciliation happens out-of-request
|
||||
// on the next scheduled vacuumScheduler run (see vacuumScheduler.ts::runNow).
|
||||
const drift = checkAutoVacuumDrift(db, settings);
|
||||
if (drift) {
|
||||
console.warn(
|
||||
`[DB] auto_vacuum drift detected (configured=${drift.configured}, live=${drift.live}); ` +
|
||||
`scheduling reconcile on the next vacuum-scheduler run`
|
||||
);
|
||||
}
|
||||
persistAutoVacuumDriftRecord(db, drift);
|
||||
}
|
||||
|
||||
export function setAutoVacuumForDb(db: SqliteDatabase, mode: AutoVacuumMode): void {
|
||||
|
||||
@@ -1,7 +1,13 @@
|
||||
import { DEFAULT_DATABASE_SETTINGS } from "@/types/databaseSettings";
|
||||
import { MAX_TIMER_TIMEOUT_MS } from "@/shared/utils/runtimeTimeouts";
|
||||
|
||||
import type { SqliteAdapter } from "./adapters/types";
|
||||
import { getDbInstance } from "./core";
|
||||
import {
|
||||
getAutoVacuumModeForDb,
|
||||
setAutoVacuumForDb,
|
||||
type AutoVacuumDrift,
|
||||
} from "./optimizationSettings";
|
||||
// Direct `key_value` access — the existing `keyValueStore` helpers only exist
|
||||
// in test fixtures; the 3 production call sites (pricingSync, jsonMigration,
|
||||
// serviceModels) all use `getDbInstance().prepare(...).run()` directly. We
|
||||
@@ -46,6 +52,10 @@ export interface VacuumSchedulerState {
|
||||
lastDurationMs: number | null;
|
||||
isRunning: boolean;
|
||||
nextRunAt: number | 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. */
|
||||
lastReclaimedPages: number | null;
|
||||
}
|
||||
|
||||
export type ScheduledVacuum = (typeof DEFAULT_DATABASE_SETTINGS)["optimization"]["scheduledVacuum"];
|
||||
@@ -73,8 +83,19 @@ const STATE_DEFAULTS: VacuumSchedulerState = {
|
||||
lastDurationMs: null,
|
||||
isRunning: false,
|
||||
nextRunAt: null,
|
||||
autoVacuumDrift: null,
|
||||
lastReclaimedPages: null,
|
||||
};
|
||||
|
||||
// Shared key_value coordinate with optimizationSettings.ts, which writes the
|
||||
// initial drift record at boot (see AUTO_VACUUM_DRIFT_NAMESPACE/KEY there).
|
||||
const AUTO_VACUUM_DRIFT_NAMESPACE = "scheduler";
|
||||
const AUTO_VACUUM_DRIFT_KEY = "vacuumDrift";
|
||||
|
||||
// Bounded per-run reclaim so a scheduled vacuum on a multi-GB INCREMENTAL
|
||||
// database never blocks for as long as a full VACUUM would (#13432 fix #2).
|
||||
const INCREMENTAL_VACUUM_BATCH_PAGES = 2000;
|
||||
|
||||
let timer: ReturnType<typeof setTimeout> | null = null;
|
||||
let currentState: VacuumSchedulerState = { ...STATE_DEFAULTS };
|
||||
|
||||
@@ -230,6 +251,35 @@ function persistState(): void {
|
||||
setKeyValue(KEY_VALUE_NAMESPACE, KEY_VALUE_KEY, JSON.stringify(currentState));
|
||||
}
|
||||
|
||||
function isAutoVacuumDrift(value: unknown): value is AutoVacuumDrift {
|
||||
return isRecord(value) && typeof value.configured === "string" && typeof value.live === "string";
|
||||
}
|
||||
|
||||
function loadAutoVacuumDrift(): AutoVacuumDrift | null {
|
||||
const raw = getKeyValue(AUTO_VACUUM_DRIFT_NAMESPACE, AUTO_VACUUM_DRIFT_KEY);
|
||||
if (!raw) return null;
|
||||
const parsed = parseJsonSafe(raw);
|
||||
return isAutoVacuumDrift(parsed) ? parsed : null;
|
||||
}
|
||||
|
||||
function clearAutoVacuumDrift(): void {
|
||||
setKeyValue(AUTO_VACUUM_DRIFT_NAMESPACE, AUTO_VACUUM_DRIFT_KEY, JSON.stringify(null));
|
||||
}
|
||||
|
||||
/**
|
||||
* Bounded reclaim step for a database already running `auto_vacuum=INCREMENTAL`:
|
||||
* frees at most `INCREMENTAL_VACUUM_BATCH_PAGES` pages per scheduled run
|
||||
* instead of the unconditional full `VACUUM` this scheduler used to always
|
||||
* issue (#13432 fix #2 / reporter's suggested fix #2). Returns the number of
|
||||
* freelist pages actually reclaimed by this batch.
|
||||
*/
|
||||
function runBoundedIncrementalVacuum(db: SqliteAdapter): number {
|
||||
const before = Number(db.pragma("freelist_count", { simple: true }) ?? 0);
|
||||
db.pragma(`incremental_vacuum(${INCREMENTAL_VACUUM_BATCH_PAGES})`);
|
||||
const after = Number(db.pragma("freelist_count", { simple: true }) ?? 0);
|
||||
return Math.max(0, before - after);
|
||||
}
|
||||
|
||||
function loadPersistedState(): Partial<VacuumSchedulerState> {
|
||||
const raw = getKeyValue(KEY_VALUE_NAMESPACE, KEY_VALUE_KEY);
|
||||
if (!raw) return {};
|
||||
@@ -252,7 +302,12 @@ export function refresh(): VacuumSchedulerState {
|
||||
return getState();
|
||||
}
|
||||
|
||||
export async function runNow(): Promise<{ success: boolean; durationMs: number; error?: string }> {
|
||||
export async function runNow(): Promise<{
|
||||
success: boolean;
|
||||
durationMs: number;
|
||||
error?: string;
|
||||
reclaimedPages?: number;
|
||||
}> {
|
||||
if (currentState.isRunning) {
|
||||
return { success: false, durationMs: 0, error: "already_running" };
|
||||
}
|
||||
@@ -262,14 +317,34 @@ export async function runNow(): Promise<{ success: boolean; durationMs: number;
|
||||
const start = Date.now();
|
||||
try {
|
||||
const db = getDbInstance();
|
||||
db.exec("VACUUM");
|
||||
let reclaimedPages: number | null = null;
|
||||
|
||||
// #13432: reconcile a configured-vs-live auto_vacuum drift first, out of
|
||||
// request handling, on this bounded/observable scheduled path — never
|
||||
// synchronously at startup (see optimizationSettings.ts::applyStoredDatabaseOptimizationSettings).
|
||||
const drift = loadAutoVacuumDrift();
|
||||
if (drift) {
|
||||
console.log(
|
||||
`[DB] Reconciling auto_vacuum drift (configured=${drift.configured}, live=${drift.live}): ` +
|
||||
`running one-time conversion VACUUM to apply the configured mode to the database file.`
|
||||
);
|
||||
setAutoVacuumForDb(db, drift.configured);
|
||||
clearAutoVacuumDrift();
|
||||
} else if (getAutoVacuumModeForDb(db) === "INCREMENTAL") {
|
||||
reclaimedPages = runBoundedIncrementalVacuum(db);
|
||||
} else {
|
||||
db.exec("VACUUM");
|
||||
}
|
||||
|
||||
const duration = Date.now() - start;
|
||||
currentState.lastRunAt = start;
|
||||
currentState.lastError = null;
|
||||
currentState.lastDurationMs = duration;
|
||||
currentState.lastReclaimedPages = reclaimedPages;
|
||||
currentState.autoVacuumDrift = loadAutoVacuumDrift();
|
||||
currentState.isRunning = false;
|
||||
refresh(); // reset the next-run clock from this successful run
|
||||
return { success: true, durationMs: duration };
|
||||
return { success: true, durationMs: duration, reclaimedPages: reclaimedPages ?? undefined };
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
currentState.lastError = message;
|
||||
@@ -296,6 +371,10 @@ export function init(): VacuumSchedulerState {
|
||||
...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(),
|
||||
};
|
||||
return refresh();
|
||||
}
|
||||
|
||||
@@ -81,6 +81,14 @@ export interface DatabaseSettings {
|
||||
lastVacuumAt: string | null;
|
||||
lastOptimizationAt: string | null;
|
||||
integrityCheck: "ok" | "error" | null;
|
||||
/**
|
||||
* #13432 — non-null while the configured `optimization.autoVacuumMode`
|
||||
* has not yet been applied to the live SQLite file. Cleared once the
|
||||
* vacuum scheduler's next scheduled run reconciles it.
|
||||
*/
|
||||
autoVacuumDrift: { configured: string; live: string } | null;
|
||||
/** Pages freed by the most recent bounded `PRAGMA incremental_vacuum` batch, or null. */
|
||||
lastReclaimedPages: number | null;
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
138
tests/unit/issue-13432-auto-vacuum-drift.test.ts
Normal file
138
tests/unit/issue-13432-auto-vacuum-drift.test.ts
Normal file
@@ -0,0 +1,138 @@
|
||||
/**
|
||||
* Regression test for #13432 — migration 046 seeds
|
||||
* `databaseSettings.autoVacuumMode = "INCREMENTAL"` into the key_value config
|
||||
* store for every database it runs against, including pre-existing ones, but
|
||||
* never touches the live SQLite `auto_vacuum` pragma (that requires a
|
||||
* `VACUUM`). The only automatic startup path
|
||||
* (`applyStoredDatabaseOptimizationSettings`) deliberately skips applying it
|
||||
* synchronously — reapplying it inline at boot would reintroduce the
|
||||
* blocking-VACUUM-on-a-multi-GB-database hazard tracked by #12821.
|
||||
*
|
||||
* Fix: detect the drift at startup (never fix it synchronously there), log +
|
||||
* persist it, and let the existing out-of-request vacuumScheduler reconcile
|
||||
* it (pragma flip + one-time conversion VACUUM) on its next scheduled run.
|
||||
* Once auto_vacuum is actually INCREMENTAL, subsequent scheduled runs use a
|
||||
* bounded `PRAGMA incremental_vacuum` batch instead of an unconditional full
|
||||
* `VACUUM`.
|
||||
*
|
||||
* DB isolation pattern mirrors tests/unit/db/vacuum-scheduler.test.ts:
|
||||
* temp DATA_DIR, resetDbInstance() before each test, 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-13432-"));
|
||||
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 optimizationSettings = await import("../../src/lib/db/optimizationSettings.ts");
|
||||
const scheduler = await import("../../src/lib/db/vacuumScheduler.ts");
|
||||
|
||||
function seedMigration046Config(mode: "NONE" | "FULL" | "INCREMENTAL") {
|
||||
// Simulate src/lib/db/migrations/046_database_settings.sql:41 running
|
||||
// against a pre-existing database: it only ever touches the config store.
|
||||
const db = core.getDbInstance();
|
||||
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)").run(
|
||||
"databaseSettings",
|
||||
"autoVacuumMode",
|
||||
JSON.stringify(mode)
|
||||
);
|
||||
}
|
||||
|
||||
function readPersistedDriftRecord(): unknown {
|
||||
const db = core.getDbInstance();
|
||||
const row = db
|
||||
.prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
|
||||
.get("scheduler", "vacuumDrift") as { value: string } | undefined;
|
||||
return row ? JSON.parse(row.value) : undefined;
|
||||
}
|
||||
|
||||
test.beforeEach(() => {
|
||||
scheduler.__resetForTests();
|
||||
const db = core.getDbInstance();
|
||||
db.prepare("DELETE FROM key_value WHERE namespace IN ('scheduler', 'databaseSettings')").run();
|
||||
});
|
||||
|
||||
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("issue #13432: startup drift detection records the mismatch WITHOUT running a blocking VACUUM", () => {
|
||||
const db = core.getDbInstance();
|
||||
|
||||
const initialLiveMode = optimizationSettings.getAutoVacuumModeForDb(db);
|
||||
assert.equal(initialLiveMode, "NONE", "sanity: fresh test DB defaults to auto_vacuum=NONE");
|
||||
|
||||
seedMigration046Config("INCREMENTAL");
|
||||
|
||||
// Simulate the next app startup (src/lib/db/core.ts calls this on every boot).
|
||||
optimizationSettings.applyStoredDatabaseOptimizationSettings(db);
|
||||
|
||||
// The core bug: the live pragma must NOT have been synchronously flipped —
|
||||
// that is the #12821 blocking-VACUUM-at-boot hazard this fix must avoid.
|
||||
assert.equal(
|
||||
optimizationSettings.getAutoVacuumModeForDb(db),
|
||||
"NONE",
|
||||
"startup path must never run a synchronous VACUUM to fix drift"
|
||||
);
|
||||
|
||||
// But the drift must now be detected and persisted for the scheduler to
|
||||
// reconcile out-of-request.
|
||||
const persisted = readPersistedDriftRecord();
|
||||
assert.deepEqual(persisted, { configured: "INCREMENTAL", live: "NONE" });
|
||||
});
|
||||
|
||||
test("issue #13432: vacuumScheduler.runNow() reconciles a pending startup drift out-of-request", async () => {
|
||||
const db = core.getDbInstance();
|
||||
seedMigration046Config("INCREMENTAL");
|
||||
optimizationSettings.applyStoredDatabaseOptimizationSettings(db);
|
||||
assert.deepEqual(readPersistedDriftRecord(), { configured: "INCREMENTAL", live: "NONE" });
|
||||
|
||||
scheduler.init();
|
||||
try {
|
||||
const result = await scheduler.runNow();
|
||||
assert.equal(result.success, true);
|
||||
|
||||
// The scheduler's bounded, out-of-request reconcile run is where the
|
||||
// one-time conversion VACUUM is allowed to happen.
|
||||
assert.equal(optimizationSettings.getAutoVacuumModeForDb(db), "INCREMENTAL");
|
||||
assert.equal(scheduler.getState().autoVacuumDrift, null);
|
||||
assert.equal(readPersistedDriftRecord(), null);
|
||||
} finally {
|
||||
scheduler.stop();
|
||||
}
|
||||
});
|
||||
|
||||
test("issue #13432: once INCREMENTAL is actually in effect and no drift remains, runNow() uses a bounded PRAGMA incremental_vacuum instead of a full VACUUM", async () => {
|
||||
const db = core.getDbInstance();
|
||||
seedMigration046Config("INCREMENTAL");
|
||||
optimizationSettings.applyStoredDatabaseOptimizationSettings(db);
|
||||
|
||||
scheduler.init();
|
||||
try {
|
||||
// First run reconciles the drift (pragma flip + one-time VACUUM).
|
||||
await scheduler.runNow();
|
||||
assert.equal(optimizationSettings.getAutoVacuumModeForDb(db), "INCREMENTAL");
|
||||
assert.equal(scheduler.getState().autoVacuumDrift, null);
|
||||
|
||||
// Second run: no drift left, live mode is INCREMENTAL — must take the
|
||||
// bounded incremental-vacuum path (reported via lastReclaimedPages),
|
||||
// never the unconditional full VACUUM this scheduler used to always run.
|
||||
const second = await scheduler.runNow();
|
||||
assert.equal(second.success, true);
|
||||
assert.equal(typeof scheduler.getState().lastReclaimedPages, "number");
|
||||
} finally {
|
||||
scheduler.stop();
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user