From 9270fcd221904c39603cf61e1397f5497773206c Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 19 Aug 2026 14:03:16 +0000 Subject: [PATCH v4] Doom the serializable transaction before raising a serialization failure A serialization failure reported against the current transaction can be caught by a subtransaction abort (ROLLBACK TO SAVEPOINT, or a PL/pgSQL exception block). Rolling back the subtransaction does not undo the reads it performed, so the dangerous structure SSI detected still exists; but since nothing was done to remember the failure, the top level transaction was free to continue and COMMIT a non-serializable result. Fix by setting SXACT_FLAG_DOOMED on the current transaction before raising the error, everywhere a serialization failure cancels the current transaction. PreCommit_CheckForSerializationFailure() then cancels the transaction at commit even if the error was swallowed. A new helper, DoomMyselfAndRaiseSerializationFailure(), makes this hard to get wrong; the places that merely re-report an already-doomed transaction use the RaiseSerializationFailure() half of it, which also deduplicates the error message boilerplate. Dooming is conservative for the failure raised during a write ("Canceled on identification as a pivot, during write"), since the rw-conflict is only recorded after that check and a rolled-back subtransaction also undoes the write. But at the time the failure is detected there is no way to know that the subtransaction will roll back, and keeping the invariant simple -- a reported serialization failure always dooms the transaction -- seems worth an occasional spurious retry in code that swallows serialization failures. The isolation test covers the original SAVEPOINT-around-a-read report, the same failure swallowed by a PL/pgSQL exception block, a failure raised during a write, and the fail-fast re-reporting on the doomed transaction's subsequent reads and writes. Author: Zsolt Parragi Author: Andrey Borodin Reported-by: Andrey Borodin Reviewed-by: Aleksander Alekseev Discussion: https://postgr.es/m/165342c0-0c75-461e-b334-b997639ad48d@aphyr.com --- src/backend/storage/lmgr/README-SSI | 11 ++ src/backend/storage/lmgr/predicate.c | 115 +++++++-------- .../expected/serializable-savepoint.out | 136 ++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/serializable-savepoint.spec | 85 +++++++++++ 5 files changed, 284 insertions(+), 64 deletions(-) create mode 100644 src/test/isolation/expected/serializable-savepoint.out create mode 100644 src/test/isolation/specs/serializable-savepoint.spec diff --git a/src/backend/storage/lmgr/README-SSI b/src/backend/storage/lmgr/README-SSI index 50d2ecca9d7..b9968e5f0fd 100644 --- a/src/backend/storage/lmgr/README-SSI +++ b/src/backend/storage/lmgr/README-SSI @@ -466,6 +466,17 @@ is based on the top level xid. When looking at an xid that comes from a tuple's xmin or xmax, for example, we always call SubTransGetTopmostTransaction() before doing much else with it. + * For the same reason, a serialization failure raised while a +subtransaction is active must doom the top level transaction: if the +error is caught by a subtransaction abort (ROLLBACK TO SAVEPOINT, or a +PL/pgSQL exception block), the reads that created the dangerous +structure are not undone, so the transaction must not be allowed to +commit. Therefore SXACT_FLAG_DOOMED is always set on the current +transaction before one of these errors is raised (see +DoomMyselfAndRaiseSerializationFailure()), so that +PreCommit_CheckForSerializationFailure() cancels the transaction at +commit even if the error was swallowed. + * PostgreSQL does not use "update in place" with a rollback log for its MVCC implementation. Where possible it uses "HOT" updates on the same page (if there is room and no indexed value is changed). diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 0ae85b7d5b4..6ebb5843dd7 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3916,6 +3916,45 @@ XidIsConcurrent(TransactionId xid) return pg_lfind32(xid, snap->xip, snap->xcnt); } +/* + * Raise a serialization failure error. + * + * The message is the same for all such failures; the caller passes the + * "Reason code" detail identifying which check failed. This is a macro so + * that ereport() takes care of formatting the detail arguments. + */ +#define RaiseSerializationFailure(...) \ + ereport(ERROR, \ + errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), \ + errmsg("could not serialize access due to read/write dependencies among transactions"), \ + errdetail_internal(__VA_ARGS__), \ + errhint("The transaction might succeed if retried.")) + +/* + * Doom the current serializable transaction and raise a serialization + * failure error. + * + * The error we raise may be caught by a subtransaction abort (ROLLBACK TO + * SAVEPOINT, or a PL/pgSQL exception block), after which the top level + * transaction could continue and even commit. That must not rescue it: + * rolling back a subtransaction does not undo the reads it performed, or + * un-observe the dangerous structure we detected (see the discussion of + * subtransactions in README-SSI). Set SXACT_FLAG_DOOMED before raising + * the error, so that PreCommit_CheckForSerializationFailure() cancels the + * transaction at commit even if the error is swallowed. + * + * The caller must hold SerializableXactHashLock exclusive; we release it + * before raising the error. The detail arguments are evaluated after the + * release, so they must not read data protected by that lock. + */ +#define DoomMyselfAndRaiseSerializationFailure(...) \ + do { \ + Assert(LWLockHeldByMeInMode(SerializableXactHashLock, LW_EXCLUSIVE)); \ + MySerializableXact->flags |= SXACT_FLAG_DOOMED; \ + LWLockRelease(SerializableXactHashLock); \ + RaiseSerializationFailure(__VA_ARGS__); \ + } while (0) + bool CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot) { @@ -3924,13 +3963,7 @@ CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot) /* Check if someone else has already decided that we need to die */ if (SxactIsDoomed(MySerializableXact)) - { - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on identification as a pivot, during conflict out checking."), - errhint("The transaction might succeed if retried."))); - } + RaiseSerializationFailure("Reason code: Canceled on identification as a pivot, during conflict out checking."); return true; } @@ -3960,13 +3993,7 @@ CheckForSerializableConflictOut(Relation relation, TransactionId xid, Snapshot s /* Check if someone else has already decided that we need to die */ if (SxactIsDoomed(MySerializableXact)) - { - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on identification as a pivot, during conflict out checking."), - errhint("The transaction might succeed if retried."))); - } + RaiseSerializationFailure("Reason code: Canceled on identification as a pivot, during conflict out checking."); Assert(TransactionIdIsValid(xid)); if (TransactionIdEquals(xid, GetTopTransactionIdIfAny())) @@ -3994,19 +4021,11 @@ CheckForSerializableConflictOut(Relation relation, TransactionId xid, Snapshot s && (!SxactIsReadOnly(MySerializableXact) || conflictCommitSeqNo <= MySerializableXact->SeqNo.lastCommitBeforeSnapshot)) - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on conflict out to old pivot %u.", xid), - errhint("The transaction might succeed if retried."))); + DoomMyselfAndRaiseSerializationFailure("Reason code: Canceled on conflict out to old pivot %u.", xid); if (SxactHasSummaryConflictIn(MySerializableXact) || !dlist_is_empty(&MySerializableXact->inConflicts)) - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on identification as a pivot, with conflict out to old committed transaction %u.", xid), - errhint("The transaction might succeed if retried."))); + DoomMyselfAndRaiseSerializationFailure("Reason code: Canceled on identification as a pivot, with conflict out to old committed transaction %u.", xid); MySerializableXact->flags |= SXACT_FLAG_SUMMARY_CONFLICT_OUT; } @@ -4039,14 +4058,7 @@ CheckForSerializableConflictOut(Relation relation, TransactionId xid, Snapshot s return; } else - { - LWLockRelease(SerializableXactHashLock); - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on conflict out to old pivot."), - errhint("The transaction might succeed if retried."))); - } + DoomMyselfAndRaiseSerializationFailure("Reason code: Canceled on conflict out to old pivot."); } /* @@ -4271,11 +4283,7 @@ CheckForSerializableConflictIn(Relation relation, const ItemPointerData *tid, Bl /* Check if someone else has already decided that we need to die */ if (SxactIsDoomed(MySerializableXact)) - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on identification as a pivot, during conflict in checking."), - errhint("The transaction might succeed if retried."))); + RaiseSerializationFailure("Reason code: Canceled on identification as a pivot, during conflict in checking."); /* * We're doing a write which might cause rw-conflicts now or later. @@ -4588,25 +4596,15 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader, * anymore, so we have to kill the reader instead. */ if (MySerializableXact == writer) - { - LWLockRelease(SerializableXactHashLock); - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on identification as a pivot, during write."), - errhint("The transaction might succeed if retried."))); - } + DoomMyselfAndRaiseSerializationFailure("Reason code: Canceled on identification as a pivot, during write."); else if (SxactIsPrepared(writer)) { - LWLockRelease(SerializableXactHashLock); + /* read this before the lock is released */ + TransactionId wtopxid = writer->topXid; /* if we're not the writer, we have to be the reader */ Assert(MySerializableXact == reader); - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on conflict out to pivot %u, during read.", writer->topXid), - errhint("The transaction might succeed if retried."))); + DoomMyselfAndRaiseSerializationFailure("Reason code: Canceled on conflict out to pivot %u, during read.", wtopxid); } writer->flags |= SXACT_FLAG_DOOMED; } @@ -4649,11 +4647,7 @@ PreCommit_CheckForSerializationFailure(void) !SxactIsPartiallyReleased(MySerializableXact)) { LWLockRelease(SerializableXactHashLock); - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on identification as a pivot, during commit attempt."), - errhint("The transaction might succeed if retried."))); + RaiseSerializationFailure("Reason code: Canceled on identification as a pivot, during commit attempt."); } dlist_foreach(near_iter, &MySerializableXact->inConflicts) @@ -4683,14 +4677,7 @@ PreCommit_CheckForSerializationFailure(void) * in that case we commit suicide instead. */ if (SxactIsPrepared(nearConflict->sxactOut)) - { - LWLockRelease(SerializableXactHashLock); - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("could not serialize access due to read/write dependencies among transactions"), - errdetail_internal("Reason code: Canceled on commit attempt with conflict in from prepared pivot."), - errhint("The transaction might succeed if retried."))); - } + DoomMyselfAndRaiseSerializationFailure("Reason code: Canceled on commit attempt with conflict in from prepared pivot."); nearConflict->sxactOut->flags |= SXACT_FLAG_DOOMED; break; } diff --git a/src/test/isolation/expected/serializable-savepoint.out b/src/test/isolation/expected/serializable-savepoint.out new file mode 100644 index 00000000000..3424b15003d --- /dev/null +++ b/src/test/isolation/expected/serializable-savepoint.out @@ -0,0 +1,136 @@ +Parsed test spec with 3 sessions + +starting permutation: r1 w2 w1 c1 sp2 r2 rb2 c2 rall +step r1: SELECT v FROM t WHERE id = 2; +v +- +0 +(1 row) + +step w2: UPDATE t SET v = 1 WHERE id = 2; +step w1: UPDATE t SET v = 1 WHERE id = 1; +step c1: COMMIT; +step sp2: SAVEPOINT f; +step r2: SELECT v FROM t WHERE id = 1; +ERROR: could not serialize access due to read/write dependencies among transactions +step rb2: ROLLBACK TO SAVEPOINT f; +step c2: COMMIT; +ERROR: could not serialize access due to read/write dependencies among transactions +step rall: SELECT id, v FROM t ORDER BY id; +id|v +--+- + 1|1 + 2|0 + 3|0 +(3 rows) + + +starting permutation: r1 w2 w1 c1 r2x c2 rall +step r1: SELECT v FROM t WHERE id = 2; +v +- +0 +(1 row) + +step w2: UPDATE t SET v = 1 WHERE id = 2; +step w1: UPDATE t SET v = 1 WHERE id = 1; +step c1: COMMIT; +s2: NOTICE: serialization failure swallowed +step r2x: DO $$ + BEGIN + PERFORM v FROM t WHERE id = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE NOTICE 'serialization failure swallowed'; + END $$; +step c2: COMMIT; +ERROR: could not serialize access due to read/write dependencies among transactions +step rall: SELECT id, v FROM t ORDER BY id; +id|v +--+- + 1|1 + 2|0 + 3|0 +(3 rows) + + +starting permutation: r2a r1 w1 c1 sp2 w2b rb2 c2 rall +step r2a: SELECT v FROM t WHERE id = 1; +v +- +0 +(1 row) + +step r1: SELECT v FROM t WHERE id = 2; +v +- +0 +(1 row) + +step w1: UPDATE t SET v = 1 WHERE id = 1; +step c1: COMMIT; +step sp2: SAVEPOINT f; +step w2b: UPDATE t SET v = 2 WHERE id = 2; +ERROR: could not serialize access due to read/write dependencies among transactions +step rb2: ROLLBACK TO SAVEPOINT f; +step c2: COMMIT; +ERROR: could not serialize access due to read/write dependencies among transactions +step rall: SELECT id, v FROM t ORDER BY id; +id|v +--+- + 1|1 + 2|0 + 3|0 +(3 rows) + + +starting permutation: r1 w2 w1 c1 sp2 r2 rb2 r2u c2 rall +step r1: SELECT v FROM t WHERE id = 2; +v +- +0 +(1 row) + +step w2: UPDATE t SET v = 1 WHERE id = 2; +step w1: UPDATE t SET v = 1 WHERE id = 1; +step c1: COMMIT; +step sp2: SAVEPOINT f; +step r2: SELECT v FROM t WHERE id = 1; +ERROR: could not serialize access due to read/write dependencies among transactions +step rb2: ROLLBACK TO SAVEPOINT f; +step r2u: SELECT v FROM t WHERE id = 3; +ERROR: could not serialize access due to read/write dependencies among transactions +step c2: COMMIT; +step rall: SELECT id, v FROM t ORDER BY id; +id|v +--+- + 1|1 + 2|0 + 3|0 +(3 rows) + + +starting permutation: r1 w2 w1 c1 sp2 r2 rb2 w2b c2 rall +step r1: SELECT v FROM t WHERE id = 2; +v +- +0 +(1 row) + +step w2: UPDATE t SET v = 1 WHERE id = 2; +step w1: UPDATE t SET v = 1 WHERE id = 1; +step c1: COMMIT; +step sp2: SAVEPOINT f; +step r2: SELECT v FROM t WHERE id = 1; +ERROR: could not serialize access due to read/write dependencies among transactions +step rb2: ROLLBACK TO SAVEPOINT f; +step w2b: UPDATE t SET v = 2 WHERE id = 2; +ERROR: could not serialize access due to read/write dependencies among transactions +step c2: COMMIT; +step rall: SELECT id, v FROM t ORDER BY id; +id|v +--+- + 1|1 + 2|0 + 3|0 +(3 rows) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 8470d50d2bc..3be663c914e 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -131,3 +131,4 @@ test: ddl-dependency-locking test: tablespace-dependency-locking test: pub-concurrent-drop test: drop-owned-grant +test: serializable-savepoint diff --git a/src/test/isolation/specs/serializable-savepoint.spec b/src/test/isolation/specs/serializable-savepoint.spec new file mode 100644 index 00000000000..bcb64bbc763 --- /dev/null +++ b/src/test/isolation/specs/serializable-savepoint.spec @@ -0,0 +1,85 @@ +# Test that a serialization failure raised inside a subtransaction cannot be +# discarded by rolling back the subtransaction: the reads that triggered it +# still happened, so the top level transaction must be doomed. +# +# s1 and s2 form the classic write-skew dangerous structure under SERIALIZABLE: +# s1 reads row 2 and writes row 1 +# s2 writes row 2 and reads row 1 +# s1 commits first. When s2 then reads row 1 it is correctly identified as the +# pivot of a dangerous structure and PostgreSQL raises +# ERROR: could not serialize access ... +# (Canceled on conflict out to pivot ..., during read). +# +# Crucially, s2's *write* to row 2 happened BEFORE the savepoint, so it is not +# undone. s2 only wraps the offending READ in a SAVEPOINT, rolls back to it +# (swallowing the error) and commits. That COMMIT must fail: allowing it +# leaves both s1's and s2's writes committed, which is the write-skew anomaly +# SSI is supposed to prevent (no serial order exists). + +setup +{ + CREATE TABLE t (id int PRIMARY KEY, v int); + INSERT INTO t VALUES (1, 0), (2, 0), (3, 0); +} + +teardown +{ + DROP TABLE t; +} + +session s1 +setup { BEGIN ISOLATION LEVEL SERIALIZABLE; } +step r1 { SELECT v FROM t WHERE id = 2; } +step w1 { UPDATE t SET v = 1 WHERE id = 1; } +step c1 { COMMIT; } + +session s2 +setup { BEGIN ISOLATION LEVEL SERIALIZABLE; } +step w2 { UPDATE t SET v = 1 WHERE id = 2; } +step r2a { SELECT v FROM t WHERE id = 1; } +step sp2 { SAVEPOINT f; } +step r2 { SELECT v FROM t WHERE id = 1; } +step r2u { SELECT v FROM t WHERE id = 3; } +step r2x { DO $$ + BEGIN + PERFORM v FROM t WHERE id = 1; + EXCEPTION WHEN serialization_failure THEN + RAISE NOTICE 'serialization failure swallowed'; + END $$; } +step w2b { UPDATE t SET v = 2 WHERE id = 2; } +step rb2 { ROLLBACK TO SAVEPOINT f; } +step c2 { COMMIT; } + +# Used to observe the final committed state. +session s3 +step rall { SELECT id, v FROM t ORDER BY id; } + +# s2 takes its snapshot at w2 (before s1 commits), writes row 2, then after s1 +# commits it reads row 1 inside a savepoint and is cancelled. After rolling +# back to the savepoint it must not be able to commit. If it does (the bug), +# rall shows both rows updated -- the non-serializable write-skew outcome. +permutation r1 w2 w1 c1 sp2 r2 rb2 c2 rall + +# Same dangerous structure, but the failing read runs inside a PL/pgSQL +# exception block, which uses a subtransaction just like a savepoint does. +# Swallowing the error must not allow the COMMIT. +permutation r1 w2 w1 c1 r2x c2 rall + +# Here the serialization failure is raised during a *write*: s2 is identified +# as a pivot when it updates row 2, which s1 read. The write itself is undone +# by the rollback to the savepoint, but the transaction is doomed anyway: when +# the failure is detected there is no way to know that the subtransaction will +# roll back, and a transaction that has swallowed a serialization failure +# cannot be assumed safe to commit. This permutation documents that +# conservative behavior. +permutation r2a r1 w1 c1 sp2 w2b rb2 c2 rall + +# A transaction that has swallowed a serialization failure stays doomed: the +# next read re-reports the failure immediately, it does not take until COMMIT. +# The read is of row 3, which nobody else touched, so it is only cancelled +# because the transaction is doomed, not by rediscovering the dangerous +# structure. +permutation r1 w2 w1 c1 sp2 r2 rb2 r2u c2 rall + +# ... and so does the next write. +permutation r1 w2 w1 c1 sp2 r2 rb2 w2b c2 rall -- 2.43.0