Avoid resetting Xmax when it's a multi with an aborted update

HeapTupleSatisfiesUpdate can very easily "forget" tuple locks while
checking the contents of a multixact and finding it contains an aborted
update, by setting the HEAP_XMAX_INVALID bit.  This would lead to
concurrent transactions not noticing any previous locks held by
transactions that might still be running, and thus being able to acquire
subsequent locks they wouldn't be normally able to acquire.

This bug was introduced in commit 1ce150b7bb; backpatch this fix to 9.3,
like that commit.

This change reverts the change to the delete-abort-savept isolation test
in 1ce150b7bb, because that behavior change was caused by this bug.

Noticed by Andres Freund while investigating a different issue reported
by Noah Misch.

diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 1ebc5ff879521d0e2efc5fe7f3964664abd2fb25..e5d0b0a666e5af315d4f4ced4c39c3bd59a9773e 100644 (file)
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -596,13 +596,26 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
                if (TransactionIdDidCommit(xmax))
                        return HeapTupleUpdated;
 
-               /* no member, even just a locker, alive anymore */
+               /*
+                * By here, the update in the Xmax is either aborted or crashed, but
+                * what about the other members?
+                */
+
                if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple)))
+               {
+                       /*
+                        * There's no member, even just a locker, alive anymore, so we can
+                        * mark the Xmax as invalid.
+                        */
                        SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
                                                InvalidTransactionId);
-
-               /* it must have aborted or crashed */
-               return HeapTupleMayBeUpdated;
+                       return HeapTupleMayBeUpdated;
+               }
+               else
+               {
+                       /* There are lockers running */
+                       return HeapTupleBeingUpdated;
+               }
        }
 
        if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple)))

diff --git a/src/test/isolation/expected/delete-abort-savept.out b/src/test/isolation/expected/delete-abort-savept.out
index 5b8c4447284e85ace3bca7406557211e0b450514..3420cf47d77334c6c8936d125c43c41d913f76bf 100644 (file)
--- a/src/test/isolation/expected/delete-abort-savept.out
+++ b/src/test/isolation/expected/delete-abort-savept.out
@@ -23,11 +23,12 @@ key            value
 step s1svp: SAVEPOINT f;
 step s1d: DELETE FROM foo;
 step s1r: ROLLBACK TO f;
-step s2l: SELECT * FROM foo FOR UPDATE;
+step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...>
+step s1c: COMMIT;
+step s2l: <... completed>
 key            value          
 
 1              1              
-step s1c: COMMIT;
 step s2c: COMMIT;
 
 starting permutation: s1l s1svp s1d s1r s2l s2c s1c
@@ -38,12 +39,8 @@ key            value
 step s1svp: SAVEPOINT f;
 step s1d: DELETE FROM foo;
 step s1r: ROLLBACK TO f;
-step s2l: SELECT * FROM foo FOR UPDATE;
-key            value          
-
-1              1              
-step s2c: COMMIT;
-step s1c: COMMIT;
+step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...>
+invalid permutation detected
 
 starting permutation: s1l s1svp s1d s2l s1r s1c s2c
 step s1l: SELECT * FROM foo FOR KEY SHARE;