Skip to content

fix(backup): bound stalled SQLite snapshots - #68868

Open
jbryce wants to merge 2 commits into
NousResearch:mainfrom
jbryce:fix/sqlite-backup-lock-timeout
Open

fix(backup): bound stalled SQLite snapshots#68868
jbryce wants to merge 2 commits into
NousResearch:mainfrom
jbryce:fix/sqlite-backup-lock-timeout

Conversation

@jbryce

@jbryce jbryce commented Jul 21, 2026

Copy link
Copy Markdown

What does this PR do?

Prevents hermes backup from blocking indefinitely when SQLite's online backup API encounters a persistently locked database.

The backup now runs in finite page steps with SQLite busy handling disabled at the connection layer, then uses a monotonic progress callback to abort only after 30 seconds without a changed remaining-page count. Healthy large backups can run longer than 30 seconds as long as they continue making progress, including when concurrent source writes restart the online backup and increase the remaining-page count.

Failed snapshots still fail closed rather than raw-copying a live database, and partial destination files are removed after both SQLite connections close.

Related Issue

Related to #61703.

This complements #61704: that PR excludes one live Chromium profile, while this change provides a generic bound for any SQLite database that stops making progress.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Update hermes_cli/backup.py to use finite SQLite backup steps and a monotonic no-progress deadline.
  • Set source and destination SQLite busy timeouts to zero so the default busy handler cannot block each progress callback for several seconds.
  • Preserve WAL-safe online snapshots and the existing _safe_copy_db(...) -> bool API.
  • Remove failed partial destinations only after source and destination connections close.
  • Add deterministic progress/restart tests plus a real BEGIN EXCLUSIVE lock regression test.

How to Test

  1. Run scripts/run_tests.sh tests/hermes_cli/test_backup.py.
  2. Confirm the real exclusive-lock regression returns within its bound and removes the staged destination.
  3. Confirm ordinary and WAL-mode SQLite copies still succeed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux 6.8.0-136-generic, Python 3.11.15

The repository's canonical targeted runner passed all 160 tests in tests/hermes_cli/test_backup.py. Ruff and git diff --check also passed. The full roughly 40,000-test suite was attempted on this host but did not complete, so the full-suite checkbox is intentionally left unchecked.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A beyond the updated function docstring
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

=== Summary: 1 files, 160 tests passed, 0 failed
All checks passed!  # Ruff

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Jul 21, 2026
@jbryce
jbryce force-pushed the fix/sqlite-backup-lock-timeout branch from 4b74f3e to c8dd54b Compare July 22, 2026 09:08
@jbryce
jbryce force-pushed the fix/sqlite-backup-lock-timeout branch from c8dd54b to b4b3ca5 Compare July 22, 2026 09:12
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused SQLite-snapshot hardening. Current main still calls conn.backup(backup_conn) without bounded page steps, a progress callback, or a timeout at hermes_cli/backup.py:266-268; the proposed helper-level change directly covers the manual, quick-snapshot, and full-zip callers (hermes_cli/backup.py:609, :1102, :1134, :1531).

The PR preserves the existing fail-closed contract and moves failed-destination cleanup until after both connections are closed. Its deterministic busy/progress tests and exclusive-lock regression are relevant coverage for the reported failure mode.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants