Skip to content

Fix installer dropping database on every upgrade#540

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/installer-data-loss-538
Mar 12, 2026
Merged

Fix installer dropping database on every upgrade#540
erikdarlingdata merged 1 commit into
devfrom
fix/installer-data-loss-538

Conversation

@erikdarlingdata
Copy link
Copy Markdown
Owner

Summary

Fixes #538. Fixes #539.

  • 00_uninstall.sql excluded from install file list — it was being picked up by the install/*.sql glob and executed as the first step of every install, silently dropping the database. Now filtered alongside 97_/99_.
  • Abort on upgrade failure — if any upgrade script fails, installation stops instead of falling through to a full reinstall over a partially-upgraded database. New CLI exit code 8 (UpgradesFailed).
  • Version detection fallback — when installation_history exists but has no SUCCESS rows (prior GUI bug), returns "1.0.0" so all idempotent upgrades are attempted rather than treating it as a fresh install.
  • Upgrade timeout: 5min → 1hrcompress_query_stats data migration timed out on 240GB+ databases at the old 300s CommandTimeout.

Root cause

PR #431 added install/00_uninstall.sql as a standalone script for SSMS users. Both installers build their file list by globbing install/*.sql sorted alphabetically — so 00_uninstall.sql became the first file executed on every install, including upgrades. Pre-release testing on small databases didn't catch it because there was no data to miss.

Test plan

  • CLI fresh install (--reinstall): verify 00_uninstall.sql does NOT appear in the execution log
  • CLI upgrade install: verify upgrades run, then install scripts start from 01_install_database.sql
  • CLI upgrade with simulated failure: verify installer aborts with exit code 8 and does NOT proceed to install
  • GUI upgrade install: same verification — upgrades then install, no 00_uninstall
  • GUI upgrade with failure: verify abort message appears, install does not proceed
  • Version detection with empty installation_history: verify "1.0.0" fallback, upgrades attempted
  • --uninstall flag still works (uses its own code path, not the file list)
  • Data survival: capture row counts before upgrade, verify equal or greater after

🤖 Generated with Claude Code

00_uninstall.sql (added in #431 for standalone uninstall) was placed in
install/ and picked up by the file glob, making it the first script
executed on every install — including upgrades. This silently dropped
the PerformanceMonitor database before recreating it empty.

Four fixes:

1. Exclude 00_* from the install file list (both CLI and GUI), matching
   the existing 97_/99_ exclusion pattern.

2. Abort installation when any upgrade script fails instead of falling
   through to the full install path over a partially-upgraded database.
   New CLI exit code 8 (UpgradesFailed).

3. Version detection fallback: when the database exists but
   installation_history has no SUCCESS rows (prior GUI bug), return
   "1.0.0" so all idempotent upgrades are attempted rather than
   treating it as a fresh install.

4. Increase upgrade script timeout from 5 minutes to 1 hour for data
   migrations on large tables (compress_query_stats on 240GB+ DBs).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@HannahVernon
Copy link
Copy Markdown
Contributor

Should we check if a database backup has occurred recently for upgrades, and warn the user with a "do you want to proceed?" message?

@erikdarlingdata
Copy link
Copy Markdown
Owner Author

@HannahVernon I've thought about things like this, including adding a backup option to the installer for the upgrade process, but there's a lot of moving parts and potential permissions issues, and of course confusion if AGs are in use and backups are being taken elsewhere. It's not impossible, but it is not something I'd feel very confident executing on.

@erikdarlingdata erikdarlingdata merged commit 93948e4 into dev Mar 12, 2026
3 checks passed
@erikdarlingdata erikdarlingdata deleted the fix/installer-data-loss-538 branch April 9, 2026 00:34
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