Hi Andrey, > > once session 2's SIREAD locks move to virtualtransaction = '-1/0', > > session 1 commits and the final count is 2. > > I reproduced this. The overlap test in CheckTargetForConflictsIn() uses > finishedBefore for a committed transaction. OldCommittedSxact has no > valid finishedBefore, so the test ignores its predicate locks. > > Each summarized predicate lock already stores the latest commitSeqNo > among the transactions folded into it. PFA a tentative patch set which > compares that with lastCommitBeforeSnapshot. A later commit means that > at least one reader represented by the lock overlapped the writer. > > The first patch adds a deterministic injection-point test, and the > second adds the fix. > > I have two buildfarm animals available for unusual configurations. One > currently runs with WAL consistency checking, and the other checks WAL > compatibility with a zero release version. I could enable > TEST_SUMMARIZE_SERIAL on one of them. I had seen a related false > negative while trying different modes around the Jepsen report, but did > not pursue it then. Your report made me return to it. I am not sure > whether the existing check-world schedules would have exposed this bug. > Would permanently testing this mode be useful, and is there any reason > not to do that?
Patch v1 needed a rebase. I reproduced the bug and verified that the patch fixes it. All in all v1 looks good to me. -- Best regards, Aleksander Alekseev
From 339ca4244a8de1166ebd00ba9c209b4edaf37e6c Mon Sep 17 00:00:00 2001 From: Andrey Borodin <[email protected]> Date: Sun, 23 Aug 2026 15:20:16 +0300 Subject: [PATCH v2 1/2] Test SSI conflict detection after summarization When SSI exhausts its transaction array, it preserves old SIREAD locks by folding them into OldCommittedSxact. Conflict detection must continue to prevent write skew after that transition, but no automated test forces this path. Add an injection point that summarizes the oldest committed serializable transaction, and use it to exercise the same write-skew schedule before and after summarization. --- src/backend/storage/lmgr/predicate.c | 8 ++ src/test/modules/injection_points/Makefile | 3 +- .../expected/serializable-summarization.out | 98 +++++++++++++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/serializable-summarization.spec | 70 +++++++++++++ 5 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 src/test/modules/injection_points/expected/serializable-summarization.out create mode 100644 src/test/modules/injection_points/specs/serializable-summarization.spec diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 0ae85b7d5b4..bc9b5191a4c 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -210,6 +210,7 @@ #include "storage/shmem.h" #include "storage/subsystems.h" #include "utils/guc_hooks.h" +#include "utils/injection_point.h" #include "utils/rel.h" #include "utils/snapmgr.h" #include "utils/wait_event.h" @@ -1730,6 +1731,13 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, */ #ifdef TEST_SUMMARIZE_SERIAL SummarizeOldestCommittedSxact(); +#endif +#ifdef USE_INJECTION_POINTS + if (IS_INJECTION_POINT_ATTACHED("serializable-summarize")) + { + INJECTION_POINT("serializable-summarize", NULL); + SummarizeOldestCommittedSxact(); + } #endif LWLockAcquire(SerializableXactHashLock, LW_EXCLUSIVE); do diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 775ad37860d..8c9635187e4 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -36,7 +36,8 @@ ISOLATION = basic \ $(WAIT_CLEANUP_TST) \ heap_lock_update \ on_conflict_probe_window \ - multixact-aborted-updater + multixact-aborted-updater \ + serializable-summarization # some isolation tests require wal_level=replica ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/injection_points/extra.conf diff --git a/src/test/modules/injection_points/expected/serializable-summarization.out b/src/test/modules/injection_points/expected/serializable-summarization.out new file mode 100644 index 00000000000..0604ac670ed --- /dev/null +++ b/src/test/modules/injection_points/expected/serializable-summarization.out @@ -0,0 +1,98 @@ +Parsed test spec with 3 sessions + +starting permutation: s1_begin s1_read s2_begin s2_read s2_write s2_commit s1_write s1_commit +step s1_begin: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s1_read: SELECT count(*) FROM summarized_write_skew WHERE v = 1; +count +----- + 0 +(1 row) + +step s2_begin: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s2_read: SELECT count(*) FROM summarized_write_skew WHERE v = 1; +count +----- + 0 +(1 row) + +step s2_write: + UPDATE summarized_write_skew + SET v = 1 + WHERE id = 1 + AND (SELECT count(*) FROM summarized_write_skew WHERE v = 1) = 0; + +step s2_commit: COMMIT; +step s1_write: + UPDATE summarized_write_skew + SET v = 1 + WHERE id = 2 + AND (SELECT count(*) FROM summarized_write_skew WHERE v = 1) = 0; + +ERROR: could not serialize access due to read/write dependencies among transactions +step s1_commit: COMMIT; + +starting permutation: s1_begin s1_read s2_begin s2_read s2_write s2_commit attach summarize detach s1_write s1_commit +step s1_begin: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s1_read: SELECT count(*) FROM summarized_write_skew WHERE v = 1; +count +----- + 0 +(1 row) + +step s2_begin: BEGIN ISOLATION LEVEL SERIALIZABLE; +step s2_read: SELECT count(*) FROM summarized_write_skew WHERE v = 1; +count +----- + 0 +(1 row) + +step s2_write: + UPDATE summarized_write_skew + SET v = 1 + WHERE id = 1 + AND (SELECT count(*) FROM summarized_write_skew WHERE v = 1) = 0; + +step s2_commit: COMMIT; +step attach: + SELECT injection_points_attach('serializable-summarize', 'notice'); + +injection_points_attach +----------------------- + +(1 row) + +s3: NOTICE: notice triggered for injection point serializable-summarize +step summarize: + BEGIN ISOLATION LEVEL SERIALIZABLE; + SELECT 1; + COMMIT; + +?column? +-------- + 1 +(1 row) + +step detach: + SELECT count(*) > 0 AS locks_summarized + FROM pg_locks + WHERE mode = 'SIReadLock' AND virtualtransaction = '-1/0'; + SELECT injection_points_detach('serializable-summarize'); + +locks_summarized +---------------- +t +(1 row) + +injection_points_detach +----------------------- + +(1 row) + +step s1_write: + UPDATE summarized_write_skew + SET v = 1 + WHERE id = 2 + AND (SELECT count(*) FROM summarized_write_skew WHERE v = 1) = 0; + +ERROR: could not serialize access due to read/write dependencies among transactions +step s1_commit: COMMIT; diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 60148f96fda..49d29dc7564 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -44,6 +44,7 @@ injection_points_isolation = [ 'heap_lock_update', 'on_conflict_probe_window', 'multixact-aborted-updater', + 'serializable-summarization', ] # wait_cleanup terminates another backend, whose FATAL message can be lost diff --git a/src/test/modules/injection_points/specs/serializable-summarization.spec b/src/test/modules/injection_points/specs/serializable-summarization.spec new file mode 100644 index 00000000000..2bc72184daf --- /dev/null +++ b/src/test/modules/injection_points/specs/serializable-summarization.spec @@ -0,0 +1,70 @@ +# Test SSI conflict detection after summarization. + +setup +{ + CREATE EXTENSION injection_points; + CREATE TABLE summarized_write_skew (id int PRIMARY KEY, v int); + INSERT INTO summarized_write_skew VALUES (1, 0), (2, 0); +} + +teardown +{ + DROP TABLE summarized_write_skew; + DROP EXTENSION injection_points; +} + +session s1 +step s1_begin { BEGIN ISOLATION LEVEL SERIALIZABLE; } +step s1_read { SELECT count(*) FROM summarized_write_skew WHERE v = 1; } +step s1_write +{ + UPDATE summarized_write_skew + SET v = 1 + WHERE id = 2 + AND (SELECT count(*) FROM summarized_write_skew WHERE v = 1) = 0; +} +step s1_commit { COMMIT; } + +session s2 +step s2_begin { BEGIN ISOLATION LEVEL SERIALIZABLE; } +step s2_read { SELECT count(*) FROM summarized_write_skew WHERE v = 1; } +step s2_write +{ + UPDATE summarized_write_skew + SET v = 1 + WHERE id = 1 + AND (SELECT count(*) FROM summarized_write_skew WHERE v = 1) = 0; +} +step s2_commit { COMMIT; } + +session s3 +step attach +{ + SELECT injection_points_attach('serializable-summarize', 'notice'); +} +step summarize +{ + BEGIN ISOLATION LEVEL SERIALIZABLE; + SELECT 1; + COMMIT; +} +step detach +{ + SELECT count(*) > 0 AS locks_summarized + FROM pg_locks + WHERE mode = 'SIReadLock' AND virtualtransaction = '-1/0'; + SELECT injection_points_detach('serializable-summarize'); +} + +# The ordinary, unsummarized conflict is a control case. +permutation + s1_begin s1_read + s2_begin s2_read s2_write s2_commit + s1_write s1_commit + +# Moving s2's SIREAD locks to OldCommittedSxact must preserve the conflict. +permutation + s1_begin s1_read + s2_begin s2_read s2_write s2_commit + attach summarize detach + s1_write s1_commit -- 2.43.0
From 4a15f53c12574130b461e386f51ca7c32150dc1f Mon Sep 17 00:00:00 2001 From: Andrey Borodin <[email protected]> Date: Sun, 23 Aug 2026 15:23:49 +0300 Subject: [PATCH v2 2/2] Detect SSI conflicts with summarized predicate locks CheckTargetForConflictsIn() uses a transaction XID horizon to decide whether a committed SIREAD lock holder overlapped the writer. The OldCommittedSxact dummy has no valid horizon, so this test ignores locks after SSI summarizes them and can allow write skew to commit. Summarized predicate locks already retain the latest commit sequence number among the transactions represented by the lock. Compare that with the writer snapshot sequence number to determine whether at least one of those transactions overlapped the writer. Back-patch to all supported branches. Discussion: https://postgr.es/m/CA%2BCOZaCtK%3DUQbeQwdAoRw27J%2B58bJBC%2ByNHP4OH2%2By_t2UtFAg%40mail.gmail.com --- src/backend/storage/lmgr/predicate.c | 35 +++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index bc9b5191a4c..9c543a7602c 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3924,6 +3924,33 @@ XidIsConcurrent(TransactionId xid) return pg_lfind32(xid, snap->xip, snap->xcnt); } +/* + * Does this predicate lock belong to a transaction which overlaps ours? + * + * Normal SERIALIZABLEXACTs retain an XID horizon for this test. The dummy + * transaction used for summarized locks has no such horizon, but each of its + * locks retains the latest commit sequence number among the transactions + * folded into it. If the latest commit occurred after our snapshot, at least + * one transaction represented by the lock overlapped ours. + */ +static bool +PredicateLockIsForOverlappingTransaction(const PREDICATELOCK *predlock) +{ + SERIALIZABLEXACT *sxact = predlock->tag.myXact; + + if (sxact == OldCommittedSxact) + { + Assert(predlock->commitSeqNo != 0); + Assert(predlock->commitSeqNo != InvalidSerCommitSeqNo); + return predlock->commitSeqNo > + MySerializableXact->SeqNo.lastCommitBeforeSnapshot; + } + + return !SxactIsCommitted(sxact) || + TransactionIdPrecedes(GetTransactionSnapshot()->xmin, + sxact->finishedBefore); +} + bool CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot) { @@ -4160,9 +4187,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag) } } else if (!SxactIsDoomed(sxact) - && (!SxactIsCommitted(sxact) - || TransactionIdPrecedes(GetTransactionSnapshot()->xmin, - sxact->finishedBefore)) + && PredicateLockIsForOverlappingTransaction(predlock) && !RWConflictExists(sxact, MySerializableXact)) { LWLockRelease(SerializableXactHashLock); @@ -4173,9 +4198,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag) * transaction may have flagged a conflict. */ if (!SxactIsDoomed(sxact) - && (!SxactIsCommitted(sxact) - || TransactionIdPrecedes(GetTransactionSnapshot()->xmin, - sxact->finishedBefore)) + && PredicateLockIsForOverlappingTransaction(predlock) && !RWConflictExists(sxact, MySerializableXact)) { FlagRWConflict(sxact, MySerializableXact); -- 2.43.0
