store: write stamp so a packed set built before a later write is never published or loaded - #89
Merged
Merged
Conversation
…ver published or loaded A reader that rebuilt the packed matrix while another process was writing could publish a set missing the writer's later records; the writer clears the cache only once per session, so every fresh process then served that partial set (#86). Every put_record/delete_record now replaces matrix/write-stamp with a fresh random token, after the record files. matrix() reads the token before listing records, stamps it into sig.json, skips publishing if it changed during the build, and _load_packed treats a sig whose stamp differs from the current one as a miss. A token, not the meta/ directory mtime, so a write in the same clock tick cannot slip through and no quiet period is needed. Sigs from an older ir carry no stamp and stay valid until the first stamped write. Closes #86 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… through - An empty/unreadable stamp (e.g. mid in-place rewrite on Windows) reads as a fresh unique value, so it matches no sig and a build does not publish. - A failed stamp write removes the stamp instead of leaving the old token a concurrent build may already hold. - The packed dir is created once per store, not on every record write. Co-Authored-By: Claude Opus 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
1 participant
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.
Closes #86
The pick
The issue listed three options. This takes the stamp option, with an explicit write-stamp token in place of the
meta/directory mtime:put_record/delete_recordreplacesmatrix/write-stampwith a fresh random token (atomic replace, after the record's files).matrix()reads the token before listing records and writes it intosig.json. If the token changed during the build, it does not publish._load_packedtreats a sig whose stamp differs from the current token as a miss.Why a token and not mtime: a token comparison has no clock resolution, so a write in the same tick as the reader's stamp cannot slip through. No quiet period (a new magic number) is needed. A reader right after
ir buildstill publishes at once, where a quiet period would have made it wait. Like mtime, it needs no cooperation from the CLI commands (unlike the write-session marker, which also needs stale-marker expiry). "Invalidate on every write" alone does not work, as the issue says.Cost: one tiny extra file write per record write, with no fsync. In isolation this made
put_recordabout 60-80% slower on Linux (3000 writes: about 1.1 s to 1.9 s). Real builds are dominated by embedding time.Compatibility: sigs from an older
ircarry no stamp. They stay valid until the first stamped write, then get rebuilt once. An older-irwriter doesn't stamp, so mixing versions keeps today's #86 behaviour, and nothing gets worse.CorpusStore.memory()(what raglab uses) is untouched.Tests
xfail(strict=True)reprotest_reader_publishing_mid_build_does_not_hide_later_writesnow passes, and the marker is removed.delete_recordinvalidates too; an unchanged corpus keeps serving its packed set (no rebuild); an unreadable/empty stamp never matches; a failed stamp write removes the stamp.matrix()sees all 40 records.test_legacy_flat_packed_layout_still_loadsremoves the stamp its ownput_recordcreated (a legacy corpus has none).test_republishing_sweeps_older_generationsexpects the stamp file to survive a clear. A test's_save_packedstub accepts the new keyword.Review
An independent refute-review found nothing blocking. Its fixes are applied in the second commit: an unreadable/empty stamp maps to a unique non-matching value, a failed stamp write removes the stamp, and the dir is created once per store. Known residual gaps: a writer killed between its meta write and its stamp write, and a power loss losing the un-fsynced stamp rename. Either can leave a set published during that one write loadable until the next write. Master has the same gap on every write.
🤖 Generated with Claude Code