Repository navigation
fix(T13299): global backup lock LOWs: clock at acquisition, abandon lost locks, sweep orphaned copies - #1965
Merged
Merged
Conversation
team-lead follow-up on #1917: VACUUM INTO is synchronous, so proper-lockfile cannot refresh the lock mtime during the copy; at stale 60_000 a copy over 60 s could be taken over as stale and a second copy start (the class #1941 fixed for exodus). Measured on SSD: 70 MB 0.29 s, 1 GB 2.4 s; slow/FUSE disks 20-50x slower. GLOBAL_BACKUP_LOCK_STALE_MS = 10 min (>= 3x worst case, EXODUS_LOCK_STALE_MS floor). onCompromised logs instead of throwing. Test: a lock whose mtime is 90 s old (a holder blocked mid-copy) is not stolen: no second backup. At stale 60_000 the test fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rded, not published review-hotfix / team-lead on #1948: logging alone could still publish a racing duplicate. The copy now lands as cleo.db.<id>.tmp; after it, createGlobalBackup's discardIf (from autoGlobalBackup) discards it when onCompromised fired or the copy outlasted GLOBAL_BACKUP_LOCK_STALE_MS (the blocking copy can hide a compromise until it ends): tmp deleted, no rename, no meta, no rotation, a warning; BackupResult.skipped says why. Test: a lock reported compromised while held publishes nothing (dir unchanged, no auto backup listed); without the discard check it fails. Timing figures: Mac17,16 (Apple M5 Pro), internal APFS SSD. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- LOW-1: the stale-window clock starts when the lock is taken, so the db-heavy admission wait counts against it. - LOW-2: a lost or outlasted lock is abandoned, never released: the new acquireAbandonableLock routes proper-lockfile through a guarded fs, so neither its release nor its exit hook removes a directory another process may own now. - LOW-3: cleo.db.<id>.tmp copies (and SQLite sidecars) a killed backup left behind are swept under the lock once older than the stale window. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #1948 (T13293). It fixes the three review-hotfix LOWs on the single-flight session-end global backup.
This branch includes #1948's commits until #1948 merges. Review only the top commit, d39da76.
lockedAt) now starts as soon astakeLockreturns, not after thedb-heavyadmission, so the admission wait counts against the window.unlock()alone would not be enough. proper-lockfile also registers asignal-exithook thatrmdirSyncs every lock it still tracks, and its release removes the directory without checking ownership.acquireAbandonableLock(store/lock.ts) passes proper-lockfile a guardedfs. Onceabandon()is called, bothrmdirandrmdirSyncare no-ops for that lock, and the release clears the refresh timer and the tracking entry.sweepOrphanedGlobalCopiesremovescleo.db.<id>.tmpcopies and their-journal/-wal/-shmfiles once they are older than the stale window. It runs under the lock, before the due check. A live copy is younger, becauseVACUUM INTOkeeps writing it.Tests
lock-abandon.test.ts: release removes the directory. After a simulated takeover,abandonleaves the other holder's directory in place, a second abandon never throws, and the lock can be taken again.backup-store-roundtrip.test.ts:.tmp/.tmp-journalfiles and keeps a live.tmpand an unrelatedother.tmp.tsc -b packages/coreis clean, and so is biome.--base origin/main, with the new LOCK_TIMEOUT throw taggednone:local-only) and 40.Task: T13299
🤖 Generated with Claude Code