public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrey Borodin <[email protected]>
To: Kyle Kingsbury <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Cc: [email protected]
Cc: Kirk Wolak <[email protected]>
Subject: Re: Possible G2-item at SERIALIZABLE
Date: Sat, 30 May 2026 23:47:00 +0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
> On 22 May 2026, at 21:44, Kyle Kingsbury <[email protected]> wrote:
>
> If y'all have any luck reproducing this, I'd love to hear about it!
It looks like I've reproduced something similar with savepoints on HEAD and
REL_18_STABLE. I can't tell if it's really all what you observe. But, probably, part of it.
If that error is raised inside a subtransaction, ROLLBACK TO SAVEPOINT swallows it
and the transaction is free to COMMIT, defeating serializable isolation.
I did not try beyond 18, but I think it always has been there.
PFA attached isolation tester and hand-wavy fix. But I suspect there are more G-items
around.
Full Jepsen test you proposed is currently tried by Nik's machines(in cc).
Best regards, Andrey Borodin.
Attachments:
[application/octet-stream] v1-0001-Add-isolation-test-SSI-serialization-failure-swal.patch (4.2K, 2-v1-0001-Add-isolation-test-SSI-serialization-failure-swal.patch)
download | inline diff:
From 5f0e1dce5974c48eaecb855d4676e497abfc0b1a Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Sat, 30 May 2026 23:07:39 +0500
Subject: [PATCH v1 1/2] Add isolation test: SSI serialization failure
swallowed by SAVEPOINT
If that error is raised inside a subtransaction, ROLLBACK TO SAVEPOINT
swallows it and the transaction is free to COMMIT, defeating
serializable isolation.
---
.../expected/serializable-savepoint.out | 25 +++++++++
src/test/isolation/isolation_schedule | 2 +
.../specs/serializable-savepoint.spec | 52 +++++++++++++++++++
3 files changed, 79 insertions(+)
create mode 100644 src/test/isolation/expected/serializable-savepoint.out
create mode 100644 src/test/isolation/specs/serializable-savepoint.spec
diff --git a/src/test/isolation/expected/serializable-savepoint.out b/src/test/isolation/expected/serializable-savepoint.out
new file mode 100644
index 00000000000..0dd4f787efd
--- /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 15c33fad4c5..62a334ad3d9 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 00000000000..de61b615529
--- /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
--
2.50.1 (Apple Git-155)
[application/octet-stream] v1-0002-Doom-serializable-transaction-before-raising-seri.patch (2.5K, 3-v1-0002-Doom-serializable-transaction-before-raising-seri.patch)
download | inline diff:
From 28762ab32aaf01a07d8394886d6464de3a91cdb7 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <[email protected]>
Date: Sat, 30 May 2026 23:07:47 +0500
Subject: [PATCH v1 2/2] Doom serializable transaction before raising
serialization error
OnConflict_CheckForSerializationFailure() cancels the current
serializable transaction with ereport(ERROR) when it is identified as a
pivot, both on the "during write" and the "during read" paths. In both
cases it released SerializableXactHashLock and raised the error without
setting SXACT_FLAG_DOOMED on MySerializableXact first; the writer is only
doomed afterwards, which the ereport() longjmp skips for the current
transaction.
If the error is caught by a subtransaction abort (ROLLBACK TO
SAVEPOINT), the transaction is not doomed and is allowed to COMMIT,
which violates serializability. Set SXACT_FLAG_DOOMED on ourselves
before raising the error so that PreCommit_CheckForSerializationFailure()
catches the transaction at commit time even if the error was swallowed.
---
src/backend/storage/lmgr/predicate.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 0ae85b7d5b4..461e899727b 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -4589,6 +4589,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 +4604,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"),
--
2.50.1 (Apple Git-155)
view thread (9+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Possible G2-item at SERIALIZABLE
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox