diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 2fedbc4c15..7cd1fdaa8a 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -4083,6 +4083,10 @@ CheckForSerializableConflictOut(bool visible, Relation relation, * while it is visible to us. The "visible" bool indicates whether the * tuple is visible to us, while HeapTupleSatisfiesVacuum checks what else * is going on with it. + * + * In the event of a concurrently inserted tuple that also happens to have + * been concurrently updated (by a separate transaction), the xmin of the + * tuple will be used -- not the updater's xid. */ htsvResult = HeapTupleSatisfiesVacuum(tuple, TransactionXmin, buffer); switch (htsvResult) @@ -4093,17 +4097,24 @@ CheckForSerializableConflictOut(bool visible, Relation relation, xid = HeapTupleHeaderGetXmin(tuple->t_data); break; case HEAPTUPLE_RECENTLY_DEAD: - if (!visible) - return; - xid = HeapTupleHeaderGetUpdateXid(tuple->t_data); - break; case HEAPTUPLE_DELETE_IN_PROGRESS: - xid = HeapTupleHeaderGetUpdateXid(tuple->t_data); + if (visible) + xid = HeapTupleHeaderGetUpdateXid(tuple->t_data); + else + xid = HeapTupleHeaderGetXmin(tuple->t_data); + + if (TransactionIdPrecedes(xid, TransactionXmin)) + { + /* This is like the HEAPTUPLE_DEAD case */ + Assert(!visible); + return; + } break; case HEAPTUPLE_INSERT_IN_PROGRESS: xid = HeapTupleHeaderGetXmin(tuple->t_data); break; case HEAPTUPLE_DEAD: + Assert(!visible); return; default: diff --git a/src/test/isolation/expected/update-conflict-out.out b/src/test/isolation/expected/update-conflict-out.out new file mode 100644 index 0000000000..32be3269b3 --- /dev/null +++ b/src/test/isolation/expected/update-conflict-out.out @@ -0,0 +1,27 @@ +Parsed test spec with 3 sessions + +starting permutation: foo_select bar_insert foo_insert foo_commit trouble_update bar_select bar_commit trouble_abort +step foo_select: SELECT * FROM txn0 WHERE id = 42; +id val + +step bar_insert: INSERT INTO txn0 SELECT 42, 'bar_insert'; +step foo_insert: INSERT INTO txn1 SELECT 7, 'foo_insert'; +step foo_commit: COMMIT; +step trouble_update: UPDATE txn1 SET val = 'add physical version for "bar_select"' WHERE id = 7; +step bar_select: SELECT * FROM txn1 WHERE id = 7; +ERROR: could not serialize access due to read/write dependencies among transactions +step bar_commit: COMMIT; +step trouble_abort: ABORT; + +starting permutation: foo_select bar_insert foo_insert foo_commit trouble_delete bar_select bar_commit trouble_abort +step foo_select: SELECT * FROM txn0 WHERE id = 42; +id val + +step bar_insert: INSERT INTO txn0 SELECT 42, 'bar_insert'; +step foo_insert: INSERT INTO txn1 SELECT 7, 'foo_insert'; +step foo_commit: COMMIT; +step trouble_delete: DELETE FROM txn1 WHERE id = 7; +step bar_select: SELECT * FROM txn1 WHERE id = 7; +ERROR: could not serialize access due to read/write dependencies among transactions +step bar_commit: COMMIT; +step trouble_abort: ABORT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index e399b02997..c34910dfb5 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -18,6 +18,7 @@ test: two-ids test: multiple-row-versions test: index-only-scan test: predicate-lock-hot-tuple +test: update-conflict-out test: deadlock-simple test: deadlock-hard test: deadlock-soft diff --git a/src/test/isolation/specs/update-conflict-out.spec b/src/test/isolation/specs/update-conflict-out.spec new file mode 100644 index 0000000000..25c27d4ca6 --- /dev/null +++ b/src/test/isolation/specs/update-conflict-out.spec @@ -0,0 +1,54 @@ +# Test for interactions between SSI's "conflict out" handling for heapam and +# concurrently updated tuple +# +# See bug report: +# https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443%40jepsen.io + +setup +{ + CREATE TABLE txn0(id int4 PRIMARY KEY, val TEXT); + CREATE TABLE txn1(id int4 PRIMARY KEY, val TEXT); +} + +teardown +{ + DROP TABLE txn0; + DROP TABLE txn1; +} + +session "foo" +setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; } +step "foo_select" { SELECT * FROM txn0 WHERE id = 42; } +step "foo_insert" { INSERT INTO txn1 SELECT 7, 'foo_insert'; } +step "foo_commit" { COMMIT; } + +session "bar" +setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; } +step "bar_select" { SELECT * FROM txn1 WHERE id = 7; } +step "bar_insert" { INSERT INTO txn0 SELECT 42, 'bar_insert'; } +step "bar_commit" { COMMIT; } + +# This session creates the conditions that confused bar's "conflict out" +# handling in old releases affected by bug: +session "trouble" +setup { BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; } +step "trouble_update" { UPDATE txn1 SET val = 'add physical version for "bar_select"' WHERE id = 7; } +step "trouble_delete" { DELETE FROM txn1 WHERE id = 7; } +step "trouble_abort" { ABORT; } + +permutation "foo_select" + "bar_insert" + "foo_insert" "foo_commit" + "trouble_update" # Updates tuple... + "bar_select" # Should observe one distinct XID per version + "bar_commit" # "bar" should fail here at the latest + "trouble_abort" + +# Same as above, but "trouble" session DELETEs this time around +permutation "foo_select" + "bar_insert" + "foo_insert" "foo_commit" + "trouble_delete" # Deletes tuple... + "bar_select" # Should observe foo's XID + "bar_commit" # "bar" should fail here at the latest + "trouble_abort"