perf(db): bound batch_results growth via pruneBatchResults retention (keep active + last-finished + last K)

Author: Sentinel-Bluebuilder

audit/continuous.js

@@ -632,6 +632,20 @@ async function _runLoop() {
             passed,
             failed,
           }, 'real');
+
+          // Bound batch_results growth: every iteration re-tests every node and
+          // writes a full row set, so the table grows unbounded without pruning.
+          // This is the safe spot — the just-finished batch is now the
+          // last-finished (kept), and the next iteration's batch hasn't started,
+          // so nothing in-flight is at risk. Pruning is housekeeping: a failure
+          // must NOT abort the audit, so it gets its own try/catch.
+          try {
+            const { pruneBatchResults } = await import('../core/db.js');
+            const { DEFAULT_BATCH_RETENTION } = await import('../core/constants.js');
+            pruneBatchResults({ keepBatches: DEFAULT_BATCH_RETENTION }, 'real');
+          } catch (e) {
+            console.error('[continuous] batch_results prune failed:', e.message);
+          }
         } catch (dbErr) {
           console.error(`[continuous] updateBatchOnFinish failed: ${dbErr.message}`);
         }

core/constants.js

@@ -88,6 +88,14 @@ export const LCD_ENDPOINTS = [
 // ─── Batch Payment ───────────────────────────────────────────────────────────
 export const BATCH_SIZE = 5;
 
+// ─── batch_results Retention ──────────────────────────────────────────────────
+// In continuous-loop mode every iteration re-tests every node and writes a full
+// set of batch_results rows, so the table grows unbounded. pruneBatchResults()
+// (core/db.js) keeps only the last N finished+active batches. 200 batches of a
+// full-network sweep is ample history for the failure-log batch-fallback while
+// keeping the table bounded.
+export const DEFAULT_BATCH_RETENTION = 200;
+
 // ─── Cache TTLs ──────────────────────────────────────────────────────────────
 export const NODE_CACHE_TTL = 5 * 60_000;       // 5 min
 export const SESSION_MAP_TTL = 120_000;          // 2 min

core/db.js

@@ -12,7 +12,7 @@ import path from 'path';
 import { mkdirSync, existsSync, writeFileSync, readFileSync } from 'fs';
 import { fileURLToPath } from 'url';
 import { countryToContinent } from './countries.js';
-import { RAW_DIR } from './constants.js';
+import { RAW_DIR, DEFAULT_BATCH_RETENTION } from './constants.js';
 
 // ─── Raw-JSON file offload ───────────────────────────────────────────────────
 // The full JSON.stringify(result) blob is no longer stored inline in the
@@ -1638,6 +1638,89 @@ export function getLastBatch(which) {
   return { batch, nodes };
 }
 
+/**
+ * Prune old batch_results (and their now-empty parent `batches` rows) so the
+ * table stays bounded under continuous-loop mode, where every iteration
+ * re-tests every node and writes a full set of rows.
+ *
+ * KEEP-SET RATIONALE — three readers must never lose their data:
+ *   (a) the ACTIVE batch (finished_at IS NULL): the continuous-loop resume path
+ *       re-seeds already-tested addresses from its batch_results (getActiveBatch
+ *       + raw SELECTs in audit/continuous.js). Pruning it mid-flight would lose
+ *       the resume seed.
+ *   (b) the single most-recent FINISHED batch (getLastBatch): drives the /live
+ *       last-completed snapshot. Pruning it would blank /live between runs.
+ *   (c) the last K batches overall (by recency): preserves recent history for
+ *       the failure-log batch-fallback in getNodeErrors and operator review.
+ * The union of these three is the keep-set. Everything else is deleted.
+ *
+ * Children (batch_results) are deleted explicitly BEFORE parents (batches) to
+ * keep `PRAGMA foreign_key_check` clean, mirroring scripts/cleanup.mjs. An
+ * active/unfinished batch row is NEVER deleted from `batches` even if it somehow
+ * fell outside the recency window (the keep-set already includes it, but the
+ * `finished_at IS NOT NULL` guard on the parent delete is a second backstop).
+ *
+ * Pruning is housekeeping — it is NOT called on the insert hot path (that would
+ * risk the active batch). audit/continuous.js calls it once per iteration,
+ * immediately after the just-finished batch becomes the last-finished one.
+ *
+ * @param {{ keepBatches?: number }} [opts] - how many recent batches to retain
+ * @param {string} [which] - DB scope (back-compat); pass ':memory:' in tests
+ * @returns {{ deletedBatchResults: number, deletedBatches: number, keptBatches: number }}
+ */
+export function pruneBatchResults({ keepBatches = DEFAULT_BATCH_RETENTION } = {}, which) {
+  const db = getDb(which);
+  const keep = Math.max(1, Number(keepBatches) || DEFAULT_BATCH_RETENTION);
+
+  return db.transaction(() => {
+    // Build the keep-set of batch ids.
+    const keepIds = new Set();
+
+    // (c) last K batches overall, by recency. `batches` is ordered by
+    // started_at everywhere else (getActiveBatch / idx_batches_started_at).
+    for (const row of db.prepare(
+      'SELECT id FROM batches ORDER BY started_at DESC LIMIT @keep',
+    ).all({ keep })) {
+      keepIds.add(row.id);
+    }
+
+    // (a) the active batch (finished_at IS NULL).
+    const active = getActiveBatch(which);
+    if (active?.batch?.id != null) keepIds.add(active.batch.id);
+
+    // (b) the most-recent finished batch.
+    const last = getLastBatch(which);
+    if (last?.batch?.id != null) keepIds.add(last.batch.id);
+
+    const ids = [...keepIds];
+    // Guard: an empty keep-set would delete everything. `keep >= 1` plus the
+    // active/last lookups mean this is only empty when there are no batches at
+    // all — in which case both deletes below are no-ops anyway, but bail early.
+    if (ids.length === 0) {
+      return { deletedBatchResults: 0, deletedBatches: 0, keptBatches: 0 };
+    }
+
+    const placeholders = ids.map((_, i) => `@k${i}`).join(',');
+    const params = {};
+    ids.forEach((id, i) => { params[`k${i}`] = id; });
+
+    // Child delete first (batch_results), then parent (batches) — never delete
+    // an unfinished/active batch row.
+    const delResults = db.prepare(
+      `DELETE FROM batch_results WHERE batch_id NOT IN (${placeholders})`,
+    ).run(params);
+    const delBatches = db.prepare(
+      `DELETE FROM batches WHERE id NOT IN (${placeholders}) AND finished_at IS NOT NULL`,
+    ).run(params);
+
+    return {
+      deletedBatchResults: delResults.changes,
+      deletedBatches: delBatches.changes,
+      keptBatches: ids.length,
+    };
+  })();
+}
+
 // ─── Close ────────────────────────────────────────────────────────────────────
 
 /**

test/db.smoke.test.js

@@ -8,7 +8,9 @@
 
 import { getDb, useDb, insertRun, updateRunOnFinish, insertResult, insertResultsBatch,
   getRun, findRunByKey, listRuns, getLatestResultPerNode,
-  getNodeHistory, getNetworkStats, insertErrorLog, getNodeErrors, closeDb } from '../core/db.js';
+  getNodeHistory, getNetworkStats, insertErrorLog, getNodeErrors, closeDb,
+  insertBatch, insertBatchResult, updateBatchOnFinish, getBatchResults,
+  getActiveBatch, getLastBatch, pruneBatchResults } from '../core/db.js';
 
 // ─── Use a fresh in-memory DB for this test run ───────────────────────────────
 // `useDb` sets the module singleton so all exported helpers target this handle.
@@ -207,6 +209,65 @@ assert(
   `log_snippet capped at <= 16384 chars (got ${capErrors[0].log_snippet?.length})`,
 );
 
+// 14. pruneBatchResults — keep active + last-finished + last K batches
+console.log('14. pruneBatchResults retention...');
+// Build 6 batches with staggered started_at so recency is unambiguous.
+// b1..b5 are finished (oldest→newest); b6 is left ACTIVE (finished_at NULL)
+// but with the OLDEST started_at so it would fall outside a recency window —
+// proving the active-batch keep is independent of the last-K window.
+const KEEP = 3;
+const batchIds = [];
+for (let i = 0; i < 6; i++) {
+  const bid = Number(insertBatch({
+    started_at:    NOW + i * 1000, // b1 oldest .. b6 newest
+    snapshot_size: 1,
+    mode:          'p2p',
+  }));
+  batchIds.push(bid);
+  // One node result per batch, distinct address so we can verify survival.
+  insertBatchResult(bid, {
+    address:    `sentnode1batch${i}`,
+    actualMbps: 10 + i,
+    testedAt:   NOW + i * 1000,
+  });
+}
+const [b1, b2, b3, b4, b5, b6] = batchIds;
+// Finish b1..b5 (oldest→newest by finished_at). Leave b6 ACTIVE.
+[b1, b2, b3, b4, b5].forEach((bid, idx) => {
+  updateBatchOnFinish(bid, { finished_at: NOW + 10_000 + idx * 1000, passed: 1, failed: 0 });
+});
+// Make b6 ACTIVE but the OLDEST by started_at so it's outside the last-K window.
+db.prepare('UPDATE batches SET started_at = @ts, finished_at = NULL WHERE id = @id')
+  .run({ ts: NOW - 100_000, id: b6 });
+
+// Sanity: active = b6, last-finished = b5 (latest finished_at).
+eq(getActiveBatch().batch.id, b6, 'active batch is b6 (finished_at NULL)');
+eq(getLastBatch().batch.id, b5, 'last-finished batch is b5');
+
+const summary = pruneBatchResults({ keepBatches: KEEP });
+// keep-set: last-K by started_at (b5,b4,b3) ∪ active(b6) ∪ last-finished(b5) = {b3,b4,b5,b6}
+eq(summary.keptBatches, 4, 'keep-set size = last-K(3) ∪ active ∪ last-finished = 4');
+
+// (a) active batch rows survive
+eq(getBatchResults(b6).results.length, 1, 'active batch (b6) rows survive');
+// (b) most-recent finished batch rows survive
+eq(getBatchResults(b5).results.length, 1, 'last-finished batch (b5) rows survive');
+// (c) batches inside the last-K window survive
+eq(getBatchResults(b4).results.length, 1, 'recent batch (b4) rows survive');
+eq(getBatchResults(b3).results.length, 1, 'recent batch (b3) rows survive');
+// (d) old batches outside the keep-set are gone (rows AND parent batch row)
+eq(getBatchResults(b2).results.length, 0, 'old batch (b2) rows pruned');
+eq(getBatchResults(b1).results.length, 0, 'old batch (b1) rows pruned');
+assert(getBatchResults(b1).batch == null, 'old batch (b1) parent row deleted');
+assert(getBatchResults(b2).batch == null, 'old batch (b2) parent row deleted');
+// Two batches deleted (b1, b2), each with one batch_results row.
+eq(summary.deletedBatches, 2, 'deletedBatches = 2 (b1, b2)');
+eq(summary.deletedBatchResults, 2, 'deletedBatchResults = 2');
+// FK integrity intact after prune.
+assert(db.prepare('PRAGMA foreign_key_check').all().length === 0, 'foreign_key_check clean after prune');
+// Active batch never deleted as a parent even if outside window — already
+// asserted via b6 survival above.
+
 // ─── Results ──────────────────────────────────────────────────────────────────
 
 console.log(`\n${'='.repeat(50)}`);