Repository navigation
fix: queries see whole transactions and one state per query (#554, #555) - #559
Merged
Merged
Conversation
Backport of the v3 fixes (PRs #556, #557) to v2.x. #554: commits added a transaction's facts to FactStorage one load_fact call at a time, each under its own write lock, so a query on another thread could see part of the transaction. FactStorage::load_facts appends a batch under one lock; execute, WriteTransaction::commit, WAL replay and BrowserDb's commit use it. #555: selective_fact_fetch read each entity/attribute lookup under its own read lock, so a commit between two lookups was seen by some patterns and not others. The three lookup bodies become functions over &FactData and FactStorage::get_facts_for_lookups runs them all under one read guard. The old methods remain for tests only. The file-backed #555 reproducer disables checkpoints: on v2.x a query that runs during a checkpoint can lose committed facts, a separate bug. Refs #554, #555 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h
This was referenced Oct 11, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This was referenced Oct 11, 2026
Merged
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.
v2.x backport of #556 (#554) and #557 (#555). Refs #554, #555. Both are data-integrity issues, so they stay open until a release ships the fix.
#554: a query can see part of a committed transaction
executetransact/retract (since v0.6.0) andWriteTransaction::commit(since v0.5.0) added facts with oneload_factcall each, each under its own write lock.FactStorage::load_factsappends the batch under one lock. It works out the new facts and their slots before changing anything, so a slot pastu32::MAXfails the whole batch, not part of it (Codex finding; v3's version has no fallible step in its loop). Used by:executeWriteTransaction::commitBrowserDb's commit#555: patterns of one query read different states
selective_fact_fetchran each lookup under its own read lock. The bodies ofget_facts_by_entity,get_facts_by_entity_attribute_indexedandget_facts_by_attributemove unchanged into free functions over&FactData.FactStorage::get_facts_for_lookupsruns a list of them under one read guard.get_facts_by_attribute's no-index fallback now reads through that guard instead of releasing it and callingget_all_facts. The three old methods remain for tests only (#[cfg(test)]).Tests
tests/snapshot_atomicity_test.rs, ported from v3 without its checkpoint test:execute,WriteTransactionand query patterns, each in memory and on file. All 6 fail onmainwithout the fix and pass with it.Two unit tests:
load_facts_appends_a_batch_and_skips_duplicatesandfacts_for_lookups_concatenates_each_lookup_in_order.1240 passing, 9 ignored. fmt and clippy clean; the wasm32 browser build checks.
Codex review: the partial-batch finding above is fixed; it also noted the races are probabilistic.
🤖 Generated with Claude Code
https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h