diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a7dffd151ab..f5db3c775c3 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1718,7 +1718,8 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
* broken.
*/
if (TransactionIdIsValid(prev_xmax) &&
- !HeapTupleUpdateXmaxMatchesXmin(prev_xmax, heapTuple->t_data))
+ !TransactionIdEquals(prev_xmax,
+ HeapTupleHeaderGetXmin(heapTuple->t_data)))
break;
/*
@@ -1887,7 +1888,7 @@ heap_get_latest_tid(Relation relation,
* tuple. Check for XMIN match.
*/
if (TransactionIdIsValid(priorXmax) &&
- !HeapTupleUpdateXmaxMatchesXmin(priorXmax, tp.t_data))
+ !TransactionIdEquals(priorXmax, HeapTupleHeaderGetXmin(tp.t_data)))
{
UnlockReleaseBuffer(buffer);
break;
@@ -1919,39 +1920,6 @@ heap_get_latest_tid(Relation relation,
} /* end of loop */
}
-/*
- * HeapTupleUpdateXmaxMatchesXmin - verify update chain xmax/xmin lineage
- *
- * Given the new version of a tuple after some update, verify whether the
- * given Xmax (corresponding to the previous version) matches the tuple's
- * Xmin, taking into account that the Xmin might have been frozen after the
- * update.
- */
-bool
-HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
-{
- TransactionId xmin = HeapTupleHeaderGetXmin(htup);
-
- /*
- * If the xmax of the old tuple is identical to the xmin of the new one,
- * it's a match.
- */
- if (TransactionIdEquals(xmax, xmin))
- return true;
-
- /*
- * When a tuple is frozen, the original Xmin is lost, but we know it's a
- * committed transaction. So unless the Xmax is InvalidXid, we don't know
- * for certain that there is a match, but there may be one; and we must
- * return true so that a HOT chain that is half-frozen can be walked
- * correctly.
- */
- if (TransactionIdEquals(xmin, FrozenTransactionId) &&
- TransactionIdIsValid(xmax))
- return true;
-
- return false;
-}
/*
* UpdateXmaxHintBits - update tuple hint bits after xmax transaction ends
@@ -5077,7 +5045,8 @@ l4:
* the end of the chain, we're done, so return success.
*/
if (TransactionIdIsValid(priorXmax) &&
- !HeapTupleUpdateXmaxMatchesXmin(priorXmax, mytup.t_data))
+ !TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data),
+ priorXmax))
{
UnlockReleaseBuffer(buf);
return HeapTupleMayBeUpdated;
@@ -5520,26 +5489,14 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
Assert(TransactionIdIsValid(xid));
/*
- * The updating transaction cannot possibly be still running, but
- * verify whether it has committed, and request to set the
- * COMMITTED flag if so. (We normally don't see any tuples in
- * this state, because they are removed by page pruning before we
- * try to freeze the page; but this can happen if the updating
- * transaction commits after the page is pruned but before
- * HeapTupleSatisfiesVacuum).
+ * If the xid is older than the cutoff, it has to have aborted,
+ * otherwise the tuple would have gotten pruned away.
*/
if (TransactionIdPrecedes(xid, cutoff_xid))
{
- if (TransactionIdDidCommit(xid))
- {
- xid = FrozenTransactionId;
- *flags = FRM_MARK_COMMITTED | FRM_RETURN_IS_XID;
- }
- else
- {
- *flags |= FRM_INVALIDATE_XMAX;
- xid = InvalidTransactionId; /* not strictly necessary */
- }
+ Assert(!TransactionIdDidCommit(xid));
+ *flags |= FRM_INVALIDATE_XMAX;
+ xid = InvalidTransactionId; /* not strictly necessary */
}
else
{
@@ -5610,17 +5567,18 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
/*
* It's an update; should we keep it? If the transaction is known
- * aborted or crashed then it's okay to ignore it, otherwise not.
+ * aborted then it's okay to ignore it, otherwise not. However,
+ * if the Xid is older than the cutoff_xid, we must remove it.
+ * Note that such an old updater cannot possibly be committed,
+ * because HeapTupleSatisfiesVacuum would have returned
+ * HEAPTUPLE_DEAD and we would not be trying to freeze the tuple.
+ *
+ * Note the TransactionIdDidAbort() test is just an optimization
+ * and not strictly necessary for correctness.
*
* As with all tuple visibility routines, it's critical to test
- * TransactionIdIsInProgress before TransactionIdDidCommit,
+ * TransactionIdIsInProgress before the transam.c routines,
* because of race conditions explained in detail in tqual.c.
- *
- * We normally don't see committed updating transactions earlier
- * than the cutoff xid, because they are removed by page pruning
- * before we try to freeze the page; but it can happen if the
- * updating transaction commits after the page is pruned but
- * before HeapTupleSatisfiesVacuum.
*/
if (TransactionIdIsCurrentTransactionId(xid) ||
TransactionIdIsInProgress(xid))
@@ -5628,33 +5586,46 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
Assert(!TransactionIdIsValid(update_xid));
update_xid = xid;
}
- else if (TransactionIdDidCommit(xid))
+ else if (!TransactionIdDidAbort(xid))
{
/*
- * The transaction committed, so we can tell caller to set
- * HEAP_XMAX_COMMITTED. (We can only do this because we know
- * the transaction is not running.)
+ * Test whether to tell caller to set HEAP_XMAX_COMMITTED
+ * while we have the Xid still in cache. Note this can only
+ * be done if the transaction is known not running.
*/
+ if (TransactionIdDidCommit(xid))
+ update_committed = true;
Assert(!TransactionIdIsValid(update_xid));
- update_committed = true;
update_xid = xid;
}
- /*
- * Not in progress, not committed -- must be aborted or crashed;
- * we can ignore it.
- */
-
/*
* If we determined that it's an Xid corresponding to an update
* that must be retained, additionally add it to the list of
- * members of the new Multi, in case we end up using that. (We
+ * members of the new Multis, in case we end up using that. (We
* might still decide to use only an update Xid and not a multi,
* but it's easier to maintain the list as we walk the old members
* list.)
+ *
+ * It is possible to end up with a very old updater Xid that
+ * crashed and thus did not mark itself as aborted in pg_clog.
+ * That would manifest as a pre-cutoff Xid. Make sure to ignore
+ * it.
*/
if (TransactionIdIsValid(update_xid))
- newmembers[nnewmembers++] = members[i];
+ {
+ if (!TransactionIdPrecedes(update_xid, cutoff_xid))
+ {
+ newmembers[nnewmembers++] = members[i];
+ }
+ else
+ {
+ /* cannot have committed: would be HEAPTUPLE_DEAD */
+ Assert(!TransactionIdDidCommit(update_xid));
+ update_xid = InvalidTransactionId;
+ update_committed = false;
+ }
+ }
}
else
{
@@ -5721,10 +5692,7 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
*
* It is assumed that the caller has checked the tuple with
* HeapTupleSatisfiesVacuum() and determined that it is not HEAPTUPLE_DEAD
- * (else we should be removing the tuple, not freezing it). However, note
- * that we don't remove HOT tuples even if they are dead, and it'd be incorrect
- * to freeze them (because that would make them visible), so we mark them as
- * update-committed, and needing further freezing later on.
+ * (else we should be removing the tuple, not freezing it).
*
* NB: cutoff_xid *must* be <= the current global xmin, to ensure that any
* XID older than it could neither be running nor seen as running by any
@@ -5835,18 +5803,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
else if (TransactionIdIsNormal(xid) &&
TransactionIdPrecedes(xid, cutoff_xid))
{
- /*
- * Must freeze regular XIDs older than the cutoff. We must not freeze
- * a HOT-updated tuple, though; doing so would bring it back to life.
- */
- if (HeapTupleHeaderIsHotUpdated(tuple))
- {
- frz->t_infomask |= HEAP_XMAX_COMMITTED;
- changed = true;
- /* must not freeze */
- }
- else
- freeze_xmax = true;
+ freeze_xmax = true;
}
if (freeze_xmax)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index a342f57f42f..9a8db74cb95 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -435,7 +435,7 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
* Check the tuple XMIN against prior XMAX, if any
*/
if (TransactionIdIsValid(priorXmax) &&
- !HeapTupleUpdateXmaxMatchesXmin(priorXmax, htup))
+ !TransactionIdEquals(HeapTupleHeaderGetXmin(htup), priorXmax))
break;
/*
@@ -774,7 +774,7 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
htup = (HeapTupleHeader) PageGetItem(page, lp);
if (TransactionIdIsValid(priorXmax) &&
- !HeapTupleUpdateXmaxMatchesXmin(priorXmax, htup))
+ !TransactionIdEquals(priorXmax, HeapTupleHeaderGetXmin(htup)))
break;
/* Remember the root line pointer for this item */
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index abef4181068..6ece3591e4f 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -1678,15 +1678,15 @@ lazy_record_dead_tuple(LVRelStats *vacrelstats,
ItemPointer itemptr)
{
/*
- * The array must never overflow, since we rely on all deletable tuples
- * being removed; inability to remove a tuple might cause an old XID to
- * persist beyond the freeze limit, which could be disastrous later on.
+ * The array shouldn't overflow under normal behavior, but perhaps it
+ * could if we are given a really small maintenance_work_mem. In that
+ * case, just forget the last few tuples (we'll get 'em next time).
*/
- if (vacrelstats->num_dead_tuples >= vacrelstats->max_dead_tuples)
- elog(ERROR, "dead tuple array overflow");
-
- vacrelstats->dead_tuples[vacrelstats->num_dead_tuples] = *itemptr;
- vacrelstats->num_dead_tuples++;
+ if (vacrelstats->num_dead_tuples < vacrelstats->max_dead_tuples)
+ {
+ vacrelstats->dead_tuples[vacrelstats->num_dead_tuples] = *itemptr;
+ vacrelstats->num_dead_tuples++;
+ }
}
/*
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 79e4701bb8a..315a555dcff 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2019,7 +2019,8 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
* buffer's content lock, since xmin never changes in an existing
* tuple.)
*/
- if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tuple.t_data))
+ if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
+ priorXmax))
{
ReleaseBuffer(buffer);
return NULL;
@@ -2137,7 +2138,8 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
/*
* As above, if xmin isn't what we're expecting, do nothing.
*/
- if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tuple.t_data))
+ if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
+ priorXmax))
{
ReleaseBuffer(buffer);
return NULL;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 93a481f4831..4a187f899cf 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -127,9 +127,6 @@ extern void heap_get_latest_tid(Relation relation, Snapshot snapshot,
ItemPointer tid);
extern void setLastTid(const ItemPointer tid);
-extern bool HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax,
- HeapTupleHeader htup);
-
extern BulkInsertState GetBulkInsertState(void);
extern void FreeBulkInsertState(BulkInsertState);
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 07cff08b337..46134c4a77e 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -30,6 +30,5 @@ test: lock-committed-keyupdate
test: update-locked-tuple
test: propagate-lock-delete
test: tuplelock-conflict
-test: freeze-the-dead
test: drop-index-concurrently-1
test: timeouts