fix: serialize first-use store initialization - #91
Open
DivyamTalwar wants to merge 1 commit into
Open
DivyamTalwar wants to merge 1 commit into
DivyamTalwar wants to merge 1 commit into
Conversation
Parallel inspect workers open independent handles to the same local store. Their check-then-insert metadata initialization can race and reject valid openers, losing baseline coverage for those inspections. Serialize in-process initialization, tolerate a competing scheme insert, and let only the inserting opener establish the legacy notice. Consume that notice atomically so concurrent readers cannot deliver it repeatedly. Normal store operations remain outside the initialization mutex. Real file-backed regressions fail on 9e41414 with duplicate metadata keys and repeated notices. They pass 20 race-enabled repetitions, preserve old snapshots, and pass the complete store and CLI race suites. Cross-process schema-lock contention is not claimed eliminated. No schema/dependency changes; only initial opens are serialized, including different paths.
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.
What and why
Closes #90.
Serialize in-process first-use migration/connection setup and make fingerprint metadata insertion race-tolerant. Only the winning initializer creates the legacy notice, and
UpgradeNoticeconsumes that notice atomically instead of a separate read/delete pair.The production mutex covers initialization only, not normal store operations. Tests preserve existing snapshot rows and cover sixteen simultaneous handles on fresh and legacy stores, plus concurrent notice consumers.
Verification
Verified commit:
8e4060b1482071877e648df39a69341f153e61b7.9e41414abed05f10ad7ff2fbe7662a5172f151f7: duplicate metadata keys reject some of sixteen valid opens; a notice is delivered two to four times.go test -race ./internal/store -run 'TestOpenConcurrentFingerprintMigration|TestUpgradeNoticeConcurrentConsumption' -count=20: passed.go test -race ./internal/store ./cmd/pgbot: passed.bash scripts/gate.sh: passed on committed HEAD, including pinned lint, tests and all six release-target builds.go test -race ./...: passed; formatting andgit diff --checkclean.Scope and risk
Initialization is serialized across store paths in the same process. This bounded startup tradeoff avoids handle-local schema/metadata races without locking ordinary data operations. Cross-process schema-lock contention is not claimed eliminated. Tests use actual file-backed SQLite; no PostgreSQL connection is required. No historical rows, schema versions, retention policy, dependencies or JSON field shapes change. Revert the commit to restore previous initialization behavior.
Checklist