Avoid update conflict out serialization anomalies.

SSI's HeapCheckForSerializableConflictOut() test failed to correctly
handle conditions involving a concurrently inserted tuple which is later
concurrently updated by a separate transaction .  A SELECT statement
that called HeapCheckForSerializableConflictOut() could end up using the
same XID (updater's XID) for both the original tuple, and the successor
tuple, missing the XID of the xact that created the original tuple
entirely.  This only happened when neither tuple from the chain was
visible to the transaction's MVCC snapshot.

The observable symptoms of this bug were subtle.  A pair of transactions
could commit, with the later transaction failing to observe the effects
of the earlier transaction (because of the confusion created by the
update to the non-visible row).  This bug dates all the way back to
commit dafaa3ef, which added SSI.

To fix, make sure that we check the xmin of concurrently inserted tuples
that happen to also have been updated concurrently.

Author: Peter Geoghegan
Reported-By: Kyle Kingsbury
Reviewed-By: Thomas Munro
Discussion: https://postgr.es/m/db7b729d-0226-d162-a126-8a8ab2dc4443@jepsen.io
Backpatch: All supported versions

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 0e3f34fc852f6301e2e0b479947906a2250a1503..f9289dea0d346da2d485ae42461bc42e1820316a 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -3913,6 +3913,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)
@@ -3923,17 +3927,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 (file)
index 0000000..32be326
--- /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 587addb42d16050fef0f415bcc1819eff1dc1dee..04fecc3b2707d2f0e18197243e6a39bcef433986 100644 (file)
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -15,6 +15,7 @@ test: two-ids
 test: multiple-row-versions
 test: index-only-scan
 test: predicate-lock-hot-tuple
+test: update-conflict-out
 test: fk-contention
 test: fk-deadlock
 test: fk-deadlock2

diff --git a/src/test/isolation/specs/update-conflict-out.spec b/src/test/isolation/specs/update-conflict-out.spec
new file mode 100644 (file)
index 0000000..25c27d4
--- /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"