diff --git a/changelog.d/fixes/compression-run-telemetry-retention-ms.md b/changelog.d/fixes/compression-run-telemetry-retention-ms.md new file mode 100644 index 0000000000..cbaedbe25e --- /dev/null +++ b/changelog.d/fixes/compression-run-telemetry-retention-ms.md @@ -0,0 +1 @@ +- **fix(db):** the `compression_run_telemetry` retention sweep now actually deletes expired rows. Its cutoff was computed in epoch seconds while the column stores epoch milliseconds, so `WHERE timestamp < cutoff` never matched and the table added by #6848 to bound `storage.sqlite` growth was unbounded in practice. Same unit mismatch as #9625, which corrected the sibling `domain_cost_history` sweep and missed this call site diff --git a/src/lib/db/cleanup.ts b/src/lib/db/cleanup.ts index 39e044b0ec..617bf4c226 100644 --- a/src/lib/db/cleanup.ts +++ b/src/lib/db/cleanup.ts @@ -344,14 +344,16 @@ export async function cleanupXpAuditLog(): Promise { /** * Clean up old compression_run_telemetry based on retention settings. (#6848) - * Uses unix-epoch `timestamp` column (INTEGER). + * The `timestamp` column stores epoch milliseconds (recordCompressionRun stamps + * Date.now()), so the cutoff must be in milliseconds to match. Same unit bug as + * domain_cost_history (#9625), which this function was missed by. */ export async function cleanupCompressionRunTelemetry(): Promise { const db = getDbInstance(); const retention = getRetentionSettings(); const retentionDays = retention.compressionRunTelemetry; - const cutoffEpoch = Math.floor(Date.now() / 1000) - retentionDays * 86_400; + const cutoffEpoch = Date.now() - retentionDays * 86_400_000; const result: CleanupResult = { deleted: 0, errors: 0 }; diff --git a/tests/unit/repro-compression-run-telemetry-ms.test.ts b/tests/unit/repro-compression-run-telemetry-ms.test.ts new file mode 100644 index 0000000000..143ef0037c --- /dev/null +++ b/tests/unit/repro-compression-run-telemetry-ms.test.ts @@ -0,0 +1,99 @@ +/** + * compression_run_telemetry cleanup cutoff unit mismatch. + * + * cleanupCompressionRunTelemetry() computed its cutoff in epoch seconds + * (Math.floor(Date.now() / 1000)) while the timestamp column stores epoch + * milliseconds, stamped by insertCompressionRunTelemetryRow() as Date.now(). + * + * This is the same defect as domain_cost_history (#9625). That fix corrected + * cleanupDomainCostHistory() ~90 lines earlier in cleanup.ts and missed this + * sibling call site, so the retention sweep added by #6848 to bound + * storage.sqlite growth has been permanently inert: a millisecond timestamp is + * ~1000x larger than a seconds cutoff, so `WHERE timestamp < cutoff` never + * matched an old row. + * + * The first test uses the REAL writer to establish the stored unit, so it + * cannot pass if the producer's format ever changes independently. + */ + +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-crt-ms-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const { cleanupCompressionRunTelemetry } = await import("../../src/lib/db/cleanup.ts"); +const { insertCompressionRunTelemetryRow } = await import( + "../../src/lib/db/compressionRunTelemetry.ts" +); +const { getDbInstance, resetDbInstance } = await import("../../src/lib/db/core.ts"); + +test.after(() => { + resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +const DAY_MS = 86_400_000; + +function seedRow(requestId: string): void { + insertCompressionRunTelemetryRow({ + requestId, + model: "m", + provider: "p", + source: "s", + tokensBefore: 100, + tokensAfter: 50, + ratio: 0.5, + }); +} + +test("the writer stamps epoch milliseconds, not seconds", () => { + seedRow("req-unit-probe"); + const db = getDbInstance()!; + const row = db + .prepare("SELECT timestamp FROM compression_run_telemetry WHERE request_id = ?") + .get("req-unit-probe") as { timestamp: number }; + + // A seconds stamp is ~1.7e9; a milliseconds stamp is ~1.7e12. + assert.ok( + row.timestamp > 1e12, + `timestamp ${row.timestamp} is not millisecond-scale; the cleanup cutoff unit must match the writer` + ); +}); + +test("cleanupCompressionRunTelemetry deletes rows older than the retention window", async () => { + const db = getDbInstance()!; + db.exec("DELETE FROM compression_run_telemetry"); + + // Insert through the real writer, then backdate to simulate age. Backdating + // preserves the producer's unit while letting the test control the age. + const now = Date.now(); + for (const id of ["old-1", "old-2", "old-3"]) { + seedRow(id); + db.prepare("UPDATE compression_run_telemetry SET timestamp = ? WHERE request_id = ?").run( + now - 40 * DAY_MS, + id + ); + } + for (const id of ["new-1", "new-2"]) { + seedRow(id); + db.prepare("UPDATE compression_run_telemetry SET timestamp = ? WHERE request_id = ?").run( + now - 5 * DAY_MS, + id + ); + } + + const result = await cleanupCompressionRunTelemetry(); + + // With the pre-fix seconds cutoff this asserted 0 deleted: the sweep was a no-op. + assert.strictEqual(result.deleted, 3, "should delete the 3 rows aged 40 days"); + assert.strictEqual(result.errors, 0); + + const remaining = db + .prepare("SELECT COUNT(*) as cnt FROM compression_run_telemetry") + .get() as { cnt: number }; + assert.strictEqual(remaining.cnt, 2, "should keep the 2 rows aged 5 days"); +}); diff --git a/tests/unit/telemetry-auto-cleanup-6848.test.ts b/tests/unit/telemetry-auto-cleanup-6848.test.ts index 2952304628..06491e2404 100644 --- a/tests/unit/telemetry-auto-cleanup-6848.test.ts +++ b/tests/unit/telemetry-auto-cleanup-6848.test.ts @@ -47,7 +47,6 @@ test.after(() => { fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); }); -const DAY_SECONDS = 86_400; const DAY_MS = 86_400_000; /** Ensure compression_run_telemetry table exists (created lazily in production). */ @@ -161,14 +160,15 @@ test("#6848 cleanupCompressionRunTelemetry: deletes rows older than retention wi ensureTelemetryTable(); const db = getDbInstance()!; const now = Date.now(); - const nowSeconds = Math.floor(now / 1000); const insert = db.prepare( "INSERT INTO compression_run_telemetry (timestamp, tokens_before, tokens_after) VALUES (?, ?, ?)" ); - insert.run(nowSeconds - 40 * DAY_SECONDS, 1000, 500); - insert.run(nowSeconds - 40 * DAY_SECONDS, 2000, 800); - insert.run(nowSeconds - 5 * DAY_SECONDS, 1500, 600); + // insertCompressionRunTelemetryRow() stamps Date.now() — epoch MILLISECONDS. + // Seeding seconds here encoded the same unit mismatch the cleanup had. + insert.run(now - 40 * DAY_MS, 1000, 500); + insert.run(now - 40 * DAY_MS, 2000, 800); + insert.run(now - 5 * DAY_MS, 1500, 600); const result = await cleanupCompressionRunTelemetry(); @@ -184,7 +184,6 @@ test("#6848 cleanupCompressionRunTelemetry: deletes rows older than retention wi test("#6848 no rows deleted when all data is within retention window (calls all 4 real functions)", async () => { ensureTelemetryTable(); const db = getDbInstance()!; - const nowSeconds = Math.floor(Date.now() / 1000); const nowMilliseconds = Date.now(); const recentISO = new Date().toISOString(); @@ -201,7 +200,7 @@ test("#6848 no rows deleted when all data is within retention window (calls all ).run("k", "a", 5, recentISO); db.prepare( "INSERT INTO compression_run_telemetry (timestamp, tokens_before, tokens_after) VALUES (?, ?, ?)" - ).run(nowSeconds - DAY_SECONDS, 100, 50); + ).run(nowMilliseconds - DAY_MS, 100, 50); const r1 = await cleanupDomainCostHistory(); const r2 = await cleanupCompressionCacheStats();