Skip to content

fix(sqlite): tag per-migration database backups with version and prevent collisions - #3785

Open
Tyagiquamar wants to merge 2 commits into
fosrl:mainfrom
Tyagiquamar:fix/sqlite-single-backup-per-upgrade
Open

Tyagiquamar wants to merge 2 commits into
fosrl:mainfrom
Tyagiquamar:fix/sqlite-single-backup-per-upgrade

Conversation

@Tyagiquamar

Copy link
Copy Markdown

By creating this pull request, I grant the project maintainers an unlimited, perpetual license to use, modify, and redistribute these contributions under any terms they choose, including both the AGPLv3 and the Fossorial Commercial license terms. I represent that I have the right to grant this license for all contributed content.

Fixes the remaining half of #3761 (the timestamp naming was fixed by #3769; this covers the duplicate backups).

Root cause

In \server/setup/migrationsSqlite.ts, \executeScripts()\ called \�ackupDb()\ inside the per-migration loop, so upgrading across N versions produced N backups, one before each migration. Each later backup contains the earlier migrations' changes, so none of them is a clean pre-upgrade restore point (and when two migrations run within the same second, the second backup silently overwrites the first under the identical timestamped name).

Fix

Take one backup before any pending migration runs. Skipped when there is nothing to run or \DISABLE_BACKUP_ON_MIGRATION\ is set, same as before. The Postgres path never took per-migration backups, so no change was needed there.

Verification (Docker, node:24.18.1-bookworm, matching CI)

Seeded a scratch sqlite db at version 1.21.0 (pending: 1.22.0, 1.23.0) and ran the real \server/setup/migrationsSqlite.ts:

  • Before: surviving backup held mid-upgrade state (\sites\ had \exitNodeSubnet, versions 1.21.0+1.22.0 recorded); new \server/setup/migrationsSqlite.backup.test.ts\ fails on unpatched code with exit 1.
  • After: exactly one backup holding the true pre-upgrade snapshot (\sites.subnet, only 1.21.0 recorded); new test passes with exit 0.
  • \ sc --noEmit\ exit 0; \prettier --check\ on both touched files exit 0; existing \server/lib/backupFileName.test.ts\ passes.
  • Note: \eslint\ cannot run in this tree at all right now (typescript-eslint does not support the repo's TS 7.0, fails identically on untouched files; fix(deps): revert typescript 7.0.2 -> 6.0.3 to fix broken ESLint #3773 tracks that separately).

Run the new test with:
px tsx server/setup/migrationsSqlite.backup.test.ts\ (after
pm ci,
pm run set:sqlite,
pm run set:oss).

@oschwartz10612

Copy link
Copy Markdown
Member

I think ideally we want to be taking a snap shot of the database once per migration still so if one migration fails you still have a log of each successful one and can choose to restore a specific version.

@Tyagiquamar

Copy link
Copy Markdown
Author

Thanks for the feedback, @oschwartz10612! That makes total sense.

I have updated the PR to keep per-migration database snapshots:

  1. Per-migration snapshots preserved: Restored the snapshot call before each migration runs, so each migration step leaves an independent restore point.
  2. Version-tagged backup names: Backup files are now named \db_YYYY-MM-DD_HH-MM-SS_v.sqlite\ (via \ ormatBackupFileName). This makes each restore point self-describing and prevents backups from overwriting each other when consecutive migrations execute within the same second.
  3. Removed redundant backup in 1.0.0-beta9: Cleaned up the legacy inline backup block from \1.0.0-beta9.ts\ so it uses the centralized runner without taking duplicate snapshots.
  4. Regression tests added: Added test coverage in \migrationsSqlite.backup.test.ts\ verifying sequential migration snapshots, distinct intermediate schema states per backup, failure recovery in later migrations, and \DISABLE_BACKUP_ON_MIGRATION\ suppression.

@Tyagiquamar Tyagiquamar changed the title fix(sqlite): take a single database backup per upgrade instead of one per migration fix(sqlite): tag per-migration database backups with version and prevent collisions Sep 18, 2026
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.

2 participants