Repository navigation
Lock a copied file only when its writers lock it - #2787
Merged
Merged
Conversation
The rehearsal copy took a shared store lock on every root file it copied, through the base RootFile.copy_group. Taking the lock creates the lock inode (store_files.py _store_lock_file), so a root holding 118,271 plain selected-tool ledger files came out of PracticeBus.select with 118,271 `.NAME.lock` files beside them, and the ledger carry counted each one as a slot (236,542). Whether a file has a store lock is a fact of its owner. The base copy_group now copies without one; the new LockedFile capability carries the shared lock for the owners whose writers take it: the LockedStore family and HistoryArchive, which locks history_sources.json itself. SQLiteFile and RegistryFile keep their own copies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trissim
marked this pull request as ready for review
October 23, 2026 14:11
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Review of a580b4f: "this file's writers hold its store lock" was stated twice. LockedStore listed both StoreLockOwner and LockedFile, and HistoryArchive took a raw _store_lock(self.path) beside its LockedFile parent, so a new StoreLockOwner could forget LockedFile and copy unlocked. LockedFile now is the store lock owner on the file side: LockedFile(RootFile, StoreLockOwner) is StoreLockOwner's only direct child, LockedStore lists only LockedFile, and HistoryArchive writes under its inherited locked(), with its place first in STORE_LOCK_ORDER (attach reads the source registry inside it). RootFile precedes StoreLockOwner because FileStore(RootFile, ABC) and DeclaredFamily's ABC otherwise have no consistent MRO; the only name both sides define is __init_subclass__, and RootFile's calls super. 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.
The rehearsal copy created a store-lock inode beside every plain file it copied.
Defect.
PracticeCopy.include(installer/practice.py:118) copies each owned file throughowner.copy_group. Undeclared files resolve toRootFile(locked_store.pyowned_files), and the baseRootFile.copy_grouptook a shared_store_lockon the source, which creates.NAME.lock(store_files.py_store_lock_file). On a root with 118,271 selected-tool ledger files,PracticeBus.selectleft 118,271 lock files, and the ledger carry (tools/cutover/carry_selected_tool_slots.py, draft #2780) counted 236,542 slots instead of 118,271. Install 26 would have done this.Fix. Whether a file has a store lock is its owner's fact (IMPL-4, half-finished family:
SQLiteFile.copy_groupalready skipped the lock, while the base assumed every file is lockable).RootFile.copy_groupcopies with no lock.LockedFile(RootFile, StoreLockOwner)is now the store lock owner on the file side, andStoreLockOwner's only direct child. It takes the shared lock around a copy, so every lock owner copies under its lock by construction.LockedStorelists onlyLockedFile.HistoryArchivebecomes aLockedFile: itsattachwrites under the inheritedlocked()instead of a raw_store_lock(self.path), and it is first inSTORE_LOCK_ORDER, sinceattachreads the source registry while holding it.RegistryFileandSQLiteFilekeep their owncopy_group.ReadLedger > ... > LockedStore > FileStore > LockedFile > JsonDocument > RootFile > StoreLockOwner > StoreOwner > DeclaredFamily > ABC.RootFilemust precedeStoreLockOwner, becauseFileStore(RootFile, ABC)andDeclaredFamily'sABCotherwise have no consistent order. The only name both sides define is__init_subclass__, andRootFile's calls super.Measured. I copied 1,000 plain ledger files plus one
ReadLedgerthroughowned_files+copy_group. Main 0240b6d left 1,000 ledger locks; this branch leaves 0. The store lock is taken in both.Test.
test_copy_locks_only_files_their_writers_lock: plain ledger files copied throughowned_files+copy_groupleave no lock beside them, and aLockedStorefile's copy still creates its lock. The old test that pinned "RootFile.copy_group refuses a lock file" now checksLockedFilerefuses it, and that the base copies a lock without creating a lock of a lock.🤖 Generated with Claude Code