Skip to content

fix(db): prune backups after health-check-repair snapshot - #13540

Closed
KooshaPari wants to merge 3 commits into
diegosouzapw:release/v3.8.51from
KooshaPari:pr/13308-backup-prune
Closed

KooshaPari wants to merge 3 commits into
diegosouzapw:release/v3.8.51from
KooshaPari:pr/13308-backup-prune

Conversation

@KooshaPari

Copy link
Copy Markdown
Contributor

Fixes #13308

Problem

The health-check-repair backup path creates a full database snapshot via VACUUM INTO on every startup but never calls the existing retention helper (cleanupDbBackups). This causes db_backups/ to grow without bound.

On one observed install this produced 10.3 GB across 18 snapshots in 8 days, while the live database was 698 MB. A perfectly healthy database still produces a full copy on every startup.

Root cause

createManagedDbBackup() in src/lib/db/core.ts creates the snapshot but has no pruning call after it. The regular backupDbFile() path in backup.ts does call cleanupDbBackups, but the health-check-repair path uses the separate createManagedDbBackup function which was missed.

Fix

Call cleanupDbBackups() after a successful snapshot in createManagedDbBackup, so the operator's maxFiles / retentionDays settings apply uniformly to all backup paths.

A dynamic import of ./backup avoids a circular dependency (since backup.ts imports from core.ts).

Changes

  • src/lib/db/core.ts: Add cleanupDbBackups() call after VACUUM INTO in createManagedDbBackup()

Validation

  • Verified that cleanupDbBackups is exported from ./backup and accepts a backupDir option
  • The dynamic import + .catch(() => {}) ensures the backup still succeeds even if pruning fails
  • No changes to backup creation logic, only post-creation cleanup

Koosha Pari added 3 commits September 13, 2026 02:31
…bility (fixes diegosouzapw#13467)

On macOS, Strategy 2 (ioreg IOPlatformUUID) resolves before Strategy 4
(os.hostname()), causing tests that mock os.hostname() to fail.

- Add DISABLE_IOREG_STRATEGY env var check in machineId.ts Strategy 2
- Set env var in the test helper to skip ioreg on all platforms
- Both tests now pass on macOS, Linux, and Windows
…ouzapw#13137)

An eval case whose upstream call failed was graded as passed because
the error string happened to match the expected pattern. Now when
metrics.error is set, result.passed is forced to false so failed
upstream calls never inflate the pass rate.
…osouzapw#13308)

The managed backup path (VACUUM INTO) creates a full database snapshot on
every startup but never calls the existing retention helper, causing
db_backups/ to grow without bound. On one observed install this produced
10.3 GB across 18 snapshots in 8 days.

Call cleanupDbBackups() after a successful snapshot so the operator's
maxFiles / retentionDays settings apply uniformly to all backup paths.
Dynamic import avoids a circular dependency with backup.ts.
Copilot AI lite review requested due to automatic review settings September 13, 2026 09:34
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks! #13404, also from you, fixes #13308 the same way with direct backupRetention imports and a test, and it is being merged. This branch uses a fire-and-forget dynamic import with a silent catch and also carries the #13539 and #13542 commits. Closing as a duplicate of #13404.

diegosouzapw pushed a commit that referenced this pull request Sep 15, 2026
…13308) (#13404)

Prunes `db_backups/` after the health-check-repair `VACUUM INTO` snapshot (#13308). That path never ran retention, so every restart of a healthy database added another full-size copy. It now uses the same `DB_BACKUP_MAX_FILES` / `DB_BACKUP_RETENTION_DAYS` limits as `backup.ts`, importing `backupRetention` directly to avoid the import cycle.

Maintainer additions: merged the current release branch and rebaselined `src/lib/db/core.ts` 1745→1767 in `file-size-baseline.json` with a dated annotation (legitimate growth). Chosen over #13540, which did the same with a fire-and-forget dynamic import.

Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green.

Thanks @KooshaPari!
Bl0ck154 pushed a commit to Bl0ck154/OmniRoute that referenced this pull request Sep 20, 2026
…iegosouzapw#13308) (diegosouzapw#13404)

Prunes `db_backups/` after the health-check-repair `VACUUM INTO` snapshot (diegosouzapw#13308). That path never ran retention, so every restart of a healthy database added another full-size copy. It now uses the same `DB_BACKUP_MAX_FILES` / `DB_BACKUP_RETENTION_DAYS` limits as `backup.ts`, importing `backupRetention` directly to avoid the import cycle.

Maintainer additions: merged the current release branch and rebaselined `src/lib/db/core.ts` 1745→1767 in `file-size-baseline.json` with a dated annotation (legitimate growth). Chosen over diegosouzapw#13540, which did the same with a fire-and-forget dynamic import.

Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green.

Thanks @KooshaPari!
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#13308) (diegosouzapw#13404)

Prunes `db_backups/` after the health-check-repair `VACUUM INTO` snapshot (diegosouzapw#13308). That path never ran retention, so every restart of a healthy database added another full-size copy. It now uses the same `DB_BACKUP_MAX_FILES` / `DB_BACKUP_RETENTION_DAYS` limits as `backup.ts`, importing `backupRetention` directly to avoid the import cycle.

Maintainer additions: merged the current release branch and rebaselined `src/lib/db/core.ts` 1745→1767 in `file-size-baseline.json` with a dated annotation (legitimate growth). Chosen over diegosouzapw#13540, which did the same with a fire-and-forget dynamic import.

Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green.

Thanks @KooshaPari!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(backend): health-check-repair backups are never pruned — the retention call is missing from that path only

2 participants