open-dsi
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
pull/31/head^2
Alvaro Herrera 8 years ago
parent d8c435e174
commit c6764eb3ae
6 changed files with 38 additions and 105 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 109
      src/backend/access/heap/heapam.c
  2. 4
      src/backend/access/heap/pruneheap.c
  3. 20
      src/backend/commands/vacuumlazy.c
  4. 6
      src/backend/executor/execMain.c
  5. 3
      src/include/access/heapam.h
  6. 1
      src/test/isolation/isolation_schedule

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

@ -2074,7 +2074,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;
/*
@ -2260,7 +2261,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;
@ -2292,50 +2293,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
@ -5755,7 +5712,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;
@ -6449,23 +6407,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
{
@ -6538,16 +6487,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))
@ -6572,6 +6518,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
@ -6650,10 +6603,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
@ -6764,22 +6714,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
Unescape 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
Unescape View File

@ -2029,17 +2029,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
Unescape 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
Unescape 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
Unescape 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

Write Preview
Loading…
Cancel
Save