aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/backend/access/heap/heapam.c21
-rw-r--r--src/backend/access/heap/pruneheap.c22
-rw-r--r--src/backend/utils/time/tqual.c93
-rw-r--r--src/test/isolation/expected/delete-abort-savept.out13
4 files changed, 75 insertions, 74 deletions
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 31496a3063e..31518d58bf9 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 fdfa37c39c4..4dada6b6d45 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 ed66c49a91f..1ebc5ff8795 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 3420cf47d77..5b8c4447284 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;