diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 31496a3063e8e3ef0bd4de20de699ffdda893882..31518d58bf97c0fdbb13667bf7bbd85417e256fb 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -3181,7 +3181,11 @@ l2: if (!HEAP_XMAX_IS_LOCKED_ONLY(oldtup.t_data->t_infomask)) update_xact = HeapTupleGetUpdateXid(oldtup.t_data); - /* there was no UPDATE in the MultiXact; or it aborted. */ + /* + * There was no UPDATE in the MultiXact; or it aborted. No + * TransactionIdIsInProgress() call needed here, since we called + * MultiXactIdWait() above. + */ if (!TransactionIdIsValid(update_xact) || TransactionIdDidAbort(update_xact)) can_continue = true; @@ -5441,6 +5445,9 @@ GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask, * Given a multixact Xmax and corresponding infomask, which does not have the * HEAP_XMAX_LOCK_ONLY bit set, obtain and return the Xid of the updating * transaction. + * + * Caller is expected to check the status of the updating transaction, if + * necessary. */ static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask) @@ -5465,19 +5472,11 @@ MultiXactIdGetUpdateXid(TransactionId xmax, uint16 t_infomask) for (i = 0; i < nmembers; i++) { /* Ignore lockers */ - if (members[i].status == MultiXactStatusForKeyShare || - members[i].status == MultiXactStatusForShare || - members[i].status == MultiXactStatusForNoKeyUpdate || - members[i].status == MultiXactStatusForUpdate) + if (!ISUPDATE_from_mxstatus(members[i].status)) continue; - /* ignore aborted transactions */ - if (TransactionIdDidAbort(members[i].xid)) - continue; - /* there should be at most one non-aborted updater */ + /* there can be at most one updater */ Assert(update_xact == InvalidTransactionId); - Assert(members[i].status == MultiXactStatusNoKeyUpdate || - members[i].status == MultiXactStatusUpdate); update_xact = members[i].xid; #ifndef USE_ASSERT_CHECKING diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index fdfa37c39c41c6c0218c1e916b0602c909e4caee..4dada6b6d45bc90e1b8826417e0a8614c34f6e2f 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -479,22 +479,12 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum, break; case HEAPTUPLE_DELETE_IN_PROGRESS: - { - TransactionId xmax; - - /* - * This tuple may soon become DEAD. Update the hint field - * so that the page is reconsidered for pruning in future. - * If there was a MultiXactId updater, and it aborted after - * HTSV checked, then we will get an invalid Xid here. - * There is no need for future pruning of the page in that - * case, so skip it. - */ - xmax = HeapTupleHeaderGetUpdateXid(htup); - if (TransactionIdIsValid(xmax)) - heap_prune_record_prunable(prstate, xmax); - } - + /* + * This tuple may soon become DEAD. Update the hint field + * so that the page is reconsidered for pruning in future. + */ + heap_prune_record_prunable(prstate, + HeapTupleHeaderGetUpdateXid(htup)); break; case HEAPTUPLE_LIVE: diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index ed66c49a91fb4a214763817cf80268945f00c03c..1ebc5ff879521d0e2efc5fe7f3964664abd2fb25 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -223,8 +223,9 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer) TransactionId xmax; xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); /* updating subtransaction must have aborted */ if (!TransactionIdIsCurrentTransactionId(xmax)) @@ -277,14 +278,17 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer) return true; xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); + if (TransactionIdIsCurrentTransactionId(xmax)) return false; if (TransactionIdIsInProgress(xmax)) return true; if (TransactionIdDidCommit(xmax)) return false; + /* it must have aborted or crashed */ return true; } @@ -497,8 +501,9 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, TransactionId xmax; xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return HeapTupleMayBeUpdated; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); /* updating subtransaction must have aborted */ if (!TransactionIdIsCurrentTransactionId(xmax)) @@ -573,14 +578,9 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, } xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - { - if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) - return HeapTupleBeingUpdated; - SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); - return HeapTupleMayBeUpdated; - } + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); if (TransactionIdIsCurrentTransactionId(xmax)) { @@ -590,13 +590,18 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, return HeapTupleInvisible; /* updated before scan started */ } - if (MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + if (TransactionIdIsInProgress(xmax)) return HeapTupleBeingUpdated; if (TransactionIdDidCommit(xmax)) return HeapTupleUpdated; + + /* no member, even just a locker, alive anymore */ + if (!MultiXactIdIsRunning(HeapTupleHeaderGetRawXmax(tuple))) + SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, + InvalidTransactionId); + /* it must have aborted or crashed */ - SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); return HeapTupleMayBeUpdated; } @@ -722,8 +727,9 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot, TransactionId xmax; xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); /* updating subtransaction must have aborted */ if (!TransactionIdIsCurrentTransactionId(xmax)) @@ -780,8 +786,10 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot, return true; xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); + if (TransactionIdIsCurrentTransactionId(xmax)) return false; if (TransactionIdIsInProgress(xmax)) @@ -791,6 +799,7 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot, } if (TransactionIdDidCommit(xmax)) return false; + /* it must have aborted or crashed */ return true; } @@ -915,8 +924,9 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot, TransactionId xmax; xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); /* updating subtransaction must have aborted */ if (!TransactionIdIsCurrentTransactionId(xmax)) @@ -975,8 +985,10 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot, Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)); xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); + if (TransactionIdIsCurrentTransactionId(xmax)) { if (HeapTupleHeaderGetCmax(tuple) >= snapshot->curcid) @@ -993,6 +1005,7 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot, return true; /* treat as still in progress */ return false; } + /* it must have aborted or crashed */ return true; } @@ -1191,8 +1204,10 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, Assert(!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)); xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return HEAPTUPLE_LIVE; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); + if (TransactionIdIsInProgress(xmax)) return HEAPTUPLE_DELETE_IN_PROGRESS; else if (TransactionIdDidCommit(xmax)) @@ -1205,8 +1220,10 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, Assert(!(tuple->t_infomask & HEAP_XMAX_COMMITTED)); xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return HEAPTUPLE_LIVE; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); + /* multi is not running -- updating xact cannot be */ Assert(!TransactionIdIsInProgress(xmax)); if (TransactionIdDidCommit(xmax)) @@ -1216,22 +1233,13 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, else return HEAPTUPLE_DEAD; } - else - { - /* - * Not in Progress, Not Committed, so either Aborted or crashed. - */ - SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); - return HEAPTUPLE_LIVE; - } /* - * Deleter committed, but perhaps it was recent enough that some open - * transactions could still see the tuple. + * Not in Progress, Not Committed, so either Aborted or crashed. + * Remove the Xmax. */ - - /* Otherwise, it's dead and removable */ - return HEAPTUPLE_DEAD; + SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, InvalidTransactionId); + return HEAPTUPLE_LIVE; } if (!(tuple->t_infomask & HEAP_XMAX_COMMITTED)) @@ -1474,8 +1482,9 @@ HeapTupleHeaderIsOnlyLocked(HeapTupleHeader tuple) /* ... but if it's a multi, then perhaps the updating Xid aborted. */ xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) /* shouldn't happen .. */ - return true; + + /* not LOCKED_ONLY, so it has to have an xmax */ + Assert(TransactionIdIsValid(xmax)); if (TransactionIdIsCurrentTransactionId(xmax)) return false; diff --git a/src/test/isolation/expected/delete-abort-savept.out b/src/test/isolation/expected/delete-abort-savept.out index 3420cf47d77334c6c8936d125c43c41d913f76bf..5b8c4447284e85ace3bca7406557211e0b450514 100644 --- a/src/test/isolation/expected/delete-abort-savept.out +++ b/src/test/isolation/expected/delete-abort-savept.out @@ -23,12 +23,11 @@ key value step s1svp: SAVEPOINT f; step s1d: DELETE FROM foo; step s1r: ROLLBACK TO f; -step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...> -step s1c: COMMIT; -step s2l: <... completed> +step s2l: SELECT * FROM foo FOR UPDATE; key value 1 1 +step s1c: COMMIT; step s2c: COMMIT; starting permutation: s1l s1svp s1d s1r s2l s2c s1c @@ -39,8 +38,12 @@ key value step s1svp: SAVEPOINT f; step s1d: DELETE FROM foo; step s1r: ROLLBACK TO f; -step s2l: SELECT * FROM foo FOR UPDATE; <waiting ...> -invalid permutation detected +step s2l: SELECT * FROM foo FOR UPDATE; +key value + +1 1 +step s2c: COMMIT; +step s1c: COMMIT; starting permutation: s1l s1svp s1d s2l s1r s1c s2c step s1l: SELECT * FROM foo FOR KEY SHARE;