Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions src/lib/db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,9 @@ describe('daemon-owned pgserve', () => {
});

describe('retention cleanup', () => {
test('retention DELETEs are present in getConnection()', () => {
test('retention DELETEs are present in runRetention()', () => {
const source = readFileSync(join(__dirname, 'db.ts'), 'utf-8');
// All 4 retention policies present
// All 4 retention policies still defined in the runRetention function
expect(source).toContain("DELETE FROM heartbeats WHERE created_at < now() - interval '7 days'");
expect(source).toContain("DELETE FROM machine_snapshots WHERE created_at < now() - interval '30 days'");
expect(source).toContain(
Expand All @@ -351,10 +351,27 @@ describe('retention cleanup', () => {
expect(source).toContain("DELETE FROM genie_runtime_events WHERE created_at < now() - interval '14 days'");
});

test('retention is guarded by retentionRan flag', () => {
test('runRetention is exported for daemon-side timer', () => {
const source = readFileSync(join(__dirname, 'db.ts'), 'utf-8');
// runRetention must be exported so scheduler-daemon can call it on its
// periodic timer (was inline-private when it ran from runPostConnectSetup).
expect(source).toContain('export async function runRetention');
});

test('runPostConnectSetup no longer invokes runRetention (Mac CPU fix A)', () => {
const source = readFileSync(join(__dirname, 'db.ts'), 'utf-8');
// The hook-dispatch cold-start fanout path must NOT trigger retention;
// every `genie hook dispatch` bun fork would otherwise issue 4 DELETEs.
// Scheduler-daemon now owns the periodic call.
const setupFnIdx = source.indexOf('async function runPostConnectSetup');
expect(setupFnIdx).toBeGreaterThan(-1);
const setupFn = source.slice(setupFnIdx, source.indexOf('\nexport async function getConnection'));
expect(setupFn).not.toContain('await runRetention(');
});

test('retentionRan flag still exists for daemon intra-process guard', () => {
const source = readFileSync(join(__dirname, 'db.ts'), 'utf-8');
expect(source).toContain('let retentionRan = false');
expect(source).toContain('!retentionRan');
expect(source).toContain('retentionRan = true');
});

Expand All @@ -373,6 +390,16 @@ describe('retention cleanup', () => {
expect(migration).toContain('DELETE FROM audit_events');
expect(migration).toContain('DELETE FROM genie_runtime_events');
});

test('scheduler-daemon owns the periodic retention timer', () => {
const daemonSource = readFileSync(join(__dirname, 'scheduler-daemon.ts'), 'utf-8');
expect(daemonSource).toContain('retentionTimer');
expect(daemonSource).toContain('runRetention');
// 1-hour cadence
expect(daemonSource).toContain('60 * 60 * 1000');
// Cleanup on stop()
expect(daemonSource).toContain('clearInterval(retentionTimer)');
});
});

describe('pool error recovery', () => {
Expand Down
25 changes: 20 additions & 5 deletions src/lib/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,18 @@ let exitHandlerRegistered = false;
/** Whether retention cleanup has already run in this process */
let retentionRan = false;

/** Prune old rows from unbounded tables. Runs once per process, non-fatal. */
async function runRetention(sql: postgres.Sql): Promise<void> {
/**
* Prune old rows from unbounded tables. Non-fatal.
*
* Exported so `scheduler-daemon` can run this on a periodic timer instead of
* inside `runPostConnectSetup` (which fires on every fresh connection — i.e.
* every `genie hook dispatch` bun fork on a Mac dev machine, hundreds per
* minute, dominant CPU consumer per the .19 Mac-CPU root-cause analysis).
*
* The `retentionRan` flag still guards intra-process double-firing for the
* daemon's own startup-then-timer path.
*/
export async function runRetention(sql: postgres.Sql): Promise<void> {
try {
await sql.unsafe(`
DELETE FROM heartbeats WHERE created_at < now() - interval '7 days';
Expand Down Expand Up @@ -662,12 +672,17 @@ async function runPostConnectSetup(client: postgres.Sql, isTestMode: boolean, ti
if (!isTestMode && needsSeed()) await runSeed(client);
const _t4 = Date.now();

if (!isTestMode && !retentionRan) await runRetention(client);
const _t5 = Date.now();
// Retention is no longer run from getConnection — it now lives on a
// periodic timer inside scheduler-daemon. Running it here meant every
// `genie hook dispatch` bun fork (hundreds/minute on a busy Mac) issued
// four DELETEs against unbounded tables, contributing to PG pool exhaustion
// ("sorry, too many clients already" in scheduler.log) and 100% CPU on Mac.
// See PR fix(db): drop runRetention from getConnection (Mac CPU fix A).
const _t5 = _t4;

if (process.env.GENIE_PROFILE_DB) {
console.error(
`[db-profile] pgserve=${timings.t1 - timings.t0}ms migrate=${_t3 - _t2}ms seed=${_t4 - _t3}ms retention=${_t5 - _t4}ms total=${_t5 - timings.t0}ms`,
`[db-profile] pgserve=${timings.t1 - timings.t0}ms migrate=${_t3 - _t2}ms seed=${_t4 - _t3}ms retention=skipped total=${_t5 - timings.t0}ms`,
);
}
}
Expand Down
39 changes: 39 additions & 0 deletions src/lib/scheduler-daemon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2201,6 +2201,11 @@ export function startDaemon(
let eventRouterHandle: EventRouterHandle | null = null;
let deliveryUnsub: (() => Promise<void>) | null = null;
let deliveryRetryTimer: ReturnType<typeof setInterval> | null = null;
// Retention DELETEs moved here from db.ts:runPostConnectSetup so they run
// once per hour from the long-lived daemon process, NOT on every `genie
// hook dispatch` bun fork. See PR fix(db): drop runRetention from
// getConnection (Mac CPU hook-dispatch fix A).
let retentionTimer: ReturnType<typeof setInterval> | null = null;

// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: stop() is a flat cleanup sequence for all daemon resources
const stop = () => {
Expand Down Expand Up @@ -2248,6 +2253,10 @@ export function startDaemon(
clearInterval(deliveryRetryTimer);
deliveryRetryTimer = null;
}
if (retentionTimer) {
clearInterval(retentionTimer);
retentionTimer = null;
}
if (deliveryUnsub) {
deliveryUnsub().catch(() => {});
deliveryUnsub = null;
Expand Down Expand Up @@ -2604,6 +2613,36 @@ export function startDaemon(
}
}, 60_000);

// Retention timer: once at daemon startup, then every hour. Runs the
// unbounded-table prune that previously fired inside getConnection (i.e.
// on every `genie hook dispatch` bun fork — hundreds/minute on Mac).
// Owned exclusively by the daemon now; safe because retention is
// idempotent + non-fatal.
const RETENTION_INTERVAL_MS = 60 * 60 * 1000; // 1 hour
const runDaemonRetention = async () => {
try {
const { getConnection, runRetention } = await import('./db.js');
const sql = await getConnection();
await runRetention(sql);
Comment on lines +2624 to +2626

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use injected DB connection for retention loop

startDaemon is designed to run with dependency overrides, but the new retention task hard-codes import('./db.js') and calls getConnection() directly instead of deps.getConnection(). In contexts that pass mocked deps (e.g., src/lib/scheduler-daemon.test.ts startDaemon tests), this bypasses the mock and can open a real DB connection or trigger db.ts auto-start behavior (genie serve) unexpectedly, which breaks isolation and introduces side effects outside the daemon under test.

Useful? React with 👍 / 👎.

deps.log({
timestamp: deps.now().toISOString(),
level: 'debug',
event: 'retention_completed',
});
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
deps.log({
timestamp: deps.now().toISOString(),
level: 'error',
event: 'retention_error',
error: message,
});
}
};
// Fire once immediately so post-restart retention isn't delayed an hour
runDaemonRetention().catch(() => {});
retentionTimer = setInterval(runDaemonRetention, RETENTION_INTERVAL_MS);

captureFallbackTimer = await initSessionCapture(deps, config);

// Initial trigger check
Expand Down
Loading