Repository navigation
fix(txn): validate predicate reads under SERIALIZABLE - #111
Merged
Merged
Conversation
SERIALIZABLE only recorded reads by atom id or slot key, so reads that went through a posting list or an index range were never validated. Write skew through an indexed predicate and phantoms through incidence or type scans committed on both sides. Scan-style reads are now recorded at the granularity they read: an atom's reverse-index postings, a type's postings, a canonical-key hash bucket, the key range of a property-index scan, and whole slots for unbounded scans (promoted to after 8192 reads in one slot). They are validated by content at commit, so checkpoints do not cause conflicts. Reads that only steer a plan (planner estimates, the single-tenant shortcut, the eager quota check whose merge re-checks at commit, HORA domain estimates) run in an explicit unrecorded scope, which also stops every pair of inserting SERIALIZABLE transactions from conflicting on the tenant usage counter.
A serializable read of a property no transaction had set returned before reading anything, so a concurrent SET of that property did not invalidate it. Reader.property now reads the atom's property record and Reader.range reads the type's atoms when the view records reads. Reader.range also scanned nothing for an unindexed JSON path, since only indexed paths have a key; it now scans the documents.
- A keyed write that left its value unchanged did not count its slot as touched, so the commit could take the fast path past a concurrent change of that key. UNSET of an absent property lost its effect this way; every slot a transaction writes now counts. - UNSET of a property name without a key reads the property record and fails at commit if another transaction set that property. - Reader.range compares only values of the bound's type, as the index does, so an unindexed scan no longer returns other types or nulls.
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.
BEGIN SERIALIZABLErecorded only reads by atom id or slot key. Reads that went through a posting list or an index range were never validated at commit, so these anomalies committed on both sides:WHERE n.oncall = true, each turns a different one off)serializable read of property-index … between 1 and 1 was invalidatedHAS(each sees the room free, eachADDs it to a different booking)serializable read of the incident edges of atom 3 was invalidatedserializable read of the atoms of type 2 in tenant 0 was invalidatedHASSNAPSHOTis unchanged: no dirty or non-repeatable reads, lost updates rejected, write skew allowed as in any snapshot isolation.How
Scan-style reads are recorded at the granularity they read and validated by content at commit:
incident,forEachIncidentEdge,incidentTree,degree);atomsOfType,typeTree,countOfType);resolve);View.nested), so changes outside the range do not conflict;atoms(),scan(slot)).More than 8192 reads in one slot collapse into one whole-slot read. Promoted postings are compared by content, so a checkpoint between read and commit does not cause a conflict (tested).
Reads that only steer a plan run in an explicit unrecorded scope (
View.unrecorded()): planner estimates, HORA domain estimates, the single-tenant shortcut inReader, and the eager quota check inSecurity.charge(the merge precondition still enforces the quota at commit). That last one also removes a false conflict between any twoSERIALIZABLEtransactions inserting in the same tenant. Results that depend on the same calls (degree(),count(*)) are still recorded.Nothing is recorded under
SNAPSHOT; it allocates nothing new.docs/transactions/transactions.mdsection 6 describes the read kinds, granularity, false positives and the unrecorded scope, with an anomaly table.Tests
SerializableTest(13): each anomaly above conflicts underSERIALIZABLE;SNAPSHOTstill allows write skew and prevents lost updates; no conflict for inserts of unrelated types, disjoint posting lists, changes outside an indexed range, or plan estimates on an index the plan did not use;degree()is still recorded. With the fix removed, 7 of them fail.SerializableReadsTest(engine, 8): every read kind on inline and promoted postings, with a checkpoint between read and commit, unchanged (commits by rebasing) and changed (conflicts); range pruning, own writes, promotion.Note for #108: it adds
View.incidentCursor, which must record the same reverse-index read. I'll update #108 on top of this branch so the two cannot merge without it.