diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 0ae85b7d5b4be..8f319b6cbb4ac 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3994,19 +3994,30 @@ CheckForSerializableConflictOut(Relation relation, TransactionId xid, Snapshot s && (!SxactIsReadOnly(MySerializableXact) || conflictCommitSeqNo <= MySerializableXact->SeqNo.lastCommitBeforeSnapshot)) + { + /* + * Doom ourselves before raising the error, so a savepoint + * cannot swallow it and let the transaction commit. + */ + MySerializableXact->flags |= SXACT_FLAG_DOOMED; 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."))); + } if (SxactHasSummaryConflictIn(MySerializableXact) || !dlist_is_empty(&MySerializableXact->inConflicts)) + { + /* See comment above: doom ourselves before raising the error. */ + MySerializableXact->flags |= SXACT_FLAG_DOOMED; 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."))); + } MySerializableXact->flags |= SXACT_FLAG_SUMMARY_CONFLICT_OUT; } @@ -4040,6 +4051,8 @@ CheckForSerializableConflictOut(Relation relation, TransactionId xid, Snapshot s } else { + /* See comment above: doom ourselves before raising the error. */ + MySerializableXact->flags |= SXACT_FLAG_DOOMED; LWLockRelease(SerializableXactHashLock); ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), @@ -4589,6 +4602,12 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader, */ if (MySerializableXact == writer) { + /* + * Mark ourselves doomed before raising the error. Otherwise a + * subtransaction abort (ROLLBACK TO SAVEPOINT) could swallow this + * error and let the transaction commit anyway, defeating SSI. + */ + MySerializableXact->flags |= SXACT_FLAG_DOOMED; LWLockRelease(SerializableXactHashLock); ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), @@ -4598,10 +4617,13 @@ OnConflict_CheckForSerializationFailure(const SERIALIZABLEXACT *reader, } else if (SxactIsPrepared(writer)) { - LWLockRelease(SerializableXactHashLock); - /* if we're not the writer, we have to be the reader */ Assert(MySerializableXact == reader); + + /* See comment above: doom ourselves before raising the error. */ + MySerializableXact->flags |= SXACT_FLAG_DOOMED; + LWLockRelease(SerializableXactHashLock); + ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access due to read/write dependencies among transactions"), diff --git a/src/test/isolation/expected/serializable-savepoint.out b/src/test/isolation/expected/serializable-savepoint.out new file mode 100644 index 0000000000000..0dd4f787efd6b --- /dev/null +++ b/src/test/isolation/expected/serializable-savepoint.out @@ -0,0 +1,25 @@ +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 +(2 rows) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 15c33fad4c5ed..62a334ad3d930 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -127,3 +127,5 @@ test: matview-write-skew test: lock-nowait test: for-portion-of test: ddl-dependency-locking + +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 0000000000000..de61b61552918 --- /dev/null +++ b/src/test/isolation/specs/serializable-savepoint.spec @@ -0,0 +1,52 @@ +# Test that a serialization failure raised while *checking* a read (the +# "conflict out to pivot, during read" cancellation) cannot be discarded by +# rolling back to a SAVEPOINT. +# +# 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); +} + +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 sp2 { SAVEPOINT f; } +step r2 { SELECT v FROM t WHERE id = 1; } +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