Doom serializable transactions before SSI errors#42
Draft
NikolayS wants to merge 1 commit into
Draft
Conversation
A serialization failure raised inside a subtransaction can be caught by ROLLBACK TO SAVEPOINT. If the top-level serializable transaction is not marked doomed before ereport(ERROR), it can later commit with writes that should have been canceled. Mark the current SerializableXact doomed before statement-time SSI serialization failures where the current transaction is the victim, and add an isolation test for the SAVEPOINT case.
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.
Summary
This draft PR preserves Andrey's deterministic
serializable-savepointreproducer and turns the local investigation into a durable branch.The bug pattern is that an SSI serialization failure can be raised inside a subtransaction and then caught by
ROLLBACK TO SAVEPOINT. If the current top-levelSERIALIZABLEXACTwas not markedSXACT_FLAG_DOOMEDbeforeereport(ERROR), the top-level transaction can still commit earlier writes even though SSI has already decided that this transaction is the victim of a dangerous structure.The new isolation test demonstrates that directly:
s1reads row 2, writes row 1, and commitss2writes row 2 before a savepoints2reads row 1 inside the savepoint and gets an SSI serialization failures2rolls back to the savepoint, preserving its earlier writes2must still fail at top-levelCOMMITWithout the fix, the test-only patch fails:
COMMITsucceeds and final state shows both writes committed (2|1).Why SAVEPOINT recovery is not enough here
Normal savepoint semantics do discard commands executed after the savepoint, so the question is fair: if the failed read were completely erased from the serializable transaction's dependency history, the surviving writes alone could be serialized.
However PostgreSQL SSI deliberately does not model predicate reads as subtransaction-local.
src/backend/storage/lmgr/README-SSIsays:So once a statement-time SSI conflict check identifies the current top-level transaction as the victim, the error needs a durable top-level marker. Otherwise
FlagRWConflict()raises before recording the conflict withSetRWConflict(),ROLLBACK TO SAVEPOINTclears the error state, and the top-level transaction can commit despite the SSI victim decision.The MVCC docs also frame SQLSTATE
40001handling as retrying the complete transaction, including the application logic that chose which SQL to issue.Fix
Mark
MySerializableXactas doomed before raising statement-time SSI serialization failures where the current transaction is the victim.This includes Andrey's original
OnConflict_CheckForSerializationFailure()paths:Canceled on identification as a pivot, during write.Canceled on conflict out to pivot ..., during read.I also widened the patch to the analogous current-victim paths in
CheckForSerializableConflictOut():Canceled on conflict out to old pivot %u.Canceled on identification as a pivot, with conflict out to old committed transaction %u.Canceled on conflict out to old pivot.for prepared summary-conflict-out pivotVerification
Local branch was based on
NikolayS/postgresmasterat:Commands run:
Observed results:
serializable-savepointallows commit afterROLLBACK TO SAVEPOINTisolation/isolation: 130 isolation subtestsRelation to issue #41
This is probably separate from Kyle's original Jepsen
INSERT ... ON CONFLICTappend anomaly unless there is another hidden savepoint path. Kyle's published command does not request savepoints, and the Jepsen-managed reproduction recorded in issue #41 was run with--no-savepoints. The workload's:on-conflictappend path also does not use the savepoint helper; that helper is for the:update-insert-updateduplicate-key path when:savepointsis enabled.So PR #42 should be treated as a deterministic SSI/SAVEPOINT finding and not as an explanation of #41. The next useful check is still to run the Jepsen-managed reproduction from #41 against this branch: if it still reproduces, #41 needs a different root cause.