database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-28 05:21:27 +03:00
Code

Revert bogus fixes of HOT-freezing bug

Browse Source 
It turns out we misdiagnosed what the real problem was.  Revert the
previous changes, because they may have worse consequences going
forward.  A better fix is forthcoming.

The simplistic test case is kept, though disabled.

Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpjelu@alap3.anarazel.de
This commit is contained in:
Alvaro Herrera 2017-11-02 15:51:05 +01:00
parent 769756fb74
commit 7a95966bc0
6 changed files with 38 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

109
src/backend/access/heap/heapam.c
View File

@ -2060,7 +2060,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;
/*
@ -2243,7 +2244,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;
@ -2275,50 +2276,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;
/*
* If the Xmin that was in effect prior to a freeze matches the Xmax,
* it's good too.
*/
if (HeapTupleHeaderXminFrozen(htup) &&
TransactionIdEquals(HeapTupleHeaderGetRawXmin(htup), xmax))
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.
*
* We no longer freeze tuples this way, but we must keep this in order to
* interpret pre-pg_upgrade pages correctly.
*/
if (TransactionIdEquals(xmin, FrozenTransactionId) &&
TransactionIdIsValid(xmax))
return true;
return false;
}
/*
* UpdateXmaxHintBits - update tuple hint bits after xmax transaction ends
@ -5736,7 +5693,8 @@ l4:
* end of the chain, we're done, so return success.
*/
if (TransactionIdIsValid(priorXmax) &&
!HeapTupleUpdateXmaxMatchesXmin(priorXmax, mytup.t_data))
!TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data),
priorXmax))
{
result = HeapTupleMayBeUpdated;
goto out_locked;
@ -6430,23 +6388,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))
*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
{
@ -6519,16 +6468,13 @@ 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.
* Note that an updater older than cutoff_xid cannot possibly be
* committed, because HeapTupleSatisfiesVacuum would have returned
* HEAPTUPLE_DEAD and we would not be trying to freeze the tuple.
*
* As with all tuple visibility routines, it's critical to test
* TransactionIdIsInProgress before TransactionIdDidCommit,
* 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))
@ -6553,6 +6499,13 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
* we can ignore it.
*/
/*
* Since the tuple wasn't marked HEAPTUPLE_DEAD by vacuum, the
* update Xid cannot possibly be older than the xid cutoff.
*/
Assert(!TransactionIdIsValid(update_xid) ||
!TransactionIdPrecedes(update_xid, cutoff_xid));
/*
* If we determined that it's an Xid corresponding to an update
* that must be retained, additionally add it to the list of
@ -6631,10 +6584,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
@ -6745,22 +6695,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
else if (TransactionIdIsNormal(xid))
{
if (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;
totally_frozen = false;
changed = true;
/* must not freeze */
}
else
freeze_xmax = true;
}
freeze_xmax = true;
else
totally_frozen = false;
}

4
src/backend/access/heap/pruneheap.c
View File

@ -473,7 +473,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;
/*
@ -813,7 +813,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 */

20
src/backend/commands/vacuumlazy.c
View File

@ -2018,17 +2018,17 @@ 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++;
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
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++;
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
vacrelstats->num_dead_tuples);
}
}
/*

6
src/backend/executor/execMain.c
View File

@ -2595,7 +2595,8 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
* atomic, and Xmin never changes in an existing tuple, except to
* invalid or frozen, and neither of those can match priorXmax.)
*/
if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tuple.t_data))
if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
priorXmax))
{
ReleaseBuffer(buffer);
return NULL;
@ -2742,7 +2743,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;

3
src/include/access/heapam.h
View File

@ -146,9 +146,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);
extern void ReleaseBulkInsertStatePin(BulkInsertState bistate);

1
src/test/isolation/isolation_schedule
View File

@ -44,7 +44,6 @@ test: update-locked-tuple
test: propagate-lock-delete
test: tuplelock-conflict
test: tuplelock-update
test: freeze-the-dead
test: nowait
test: nowait-2
test: nowait-3