10000 Fix traversal of half-frozen update chains · abhinav-upadhyay/postgres@fc0df3b · GitHub
[go: up one dir, main page]
Skip to content

Commit fc0df3b

Browse files
alvherrealvherre
committed
Fix traversal of half-frozen update chains
When some tuple versions in an update chain are frozen due to them being older than freeze_min_age, the xmax/xmin trail can become broken. This breaks HOT (and probably other things). A subsequent VACUUM can break things in more serious ways, such as leaving orphan heap-only tuples whose root HOT redirect items were removed. This can be seen because index creation (or REINDEX) complain like ERROR: XX000: failed to find parent tuple for heap-only tuple at (0,7) in table "t" Because of relfrozenxid contraints, we cannot avoid the freezing of the early tuples, so we must cope with the results: whenever we see an Xmin of FrozenTransactionId, consider it a match for whatever the previous Xmax value was. This problem seems to have appeared in 9.3 with multixact changes, though strictly speaking it seems unrelated. Since 9.4 we have commit 37484ad "Change the way we mark tuples as frozen", so the fix is simple: just compare the raw Xmin (still stored in the tuple header, since freezing merely set an infomask bit) to the Xmax. But in 9.3 we rewrite the Xmin value to FrozenTransactionId, so the original value is lost and we have nothing to compare the Xmax with. To cope with that case we need to compare the Xmin with FrozenXid, assume it's a match, and hope for the best. Sadly, since you can pg_upgrade a 9.3 instance containing half-frozen pages to newer releases, we need to keep the old check in newer versions too, which seems a bit brittle; I hope we can somehow get rid of that. I didn't optimize the new function for performance. The new coding is probably a bit slower than before, since there is a function call rather than a straight comparison, but I'd rather have it work correctly than be fast but wrong. This is a followup after 20b6552 fixed a few related problems. Apparently, in 9.6 and up there are more ways to get into trouble, but in 9.3 - 9.5 I cannot reproduce a problem anymore with this patch, so there must be a separate bug. Reported-by: Peter Geoghegan Diagnosed-by: Peter Geoghegan, Michael Paquier, Daniel Wood, Yi Wen Wong, Álvaro Discussion: https://postgr.es/m/CAH2-Wznm4rCrhFAiwKPWTpEw2bXDtgROZK7jWWGucXeH3D1fmA@mail.gmail.com
1 parent 32022e3 commit fc0df3b

File tree

4 files changed

+54
-11
lines changed

4 files changed

+54
-11
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,8 +1824,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
18241824
* broken.
18251825
*/
18261826
if (TransactionIdIsValid(prev_xmax) &&
1827-
!TransactionIdEquals(prev_xmax,
1828-
HeapTupleHeaderGetXmin(heapTuple->t_data)))
1827+
!HeapTupleUpdateXmaxMatchesXmin(prev_xmax, heapTuple->t_data))
18291828
break;
18301829

18311830
/*
@@ -2007,7 +2006,7 @@ heap_get_latest_tid(Relation relation,
20072006
* tuple. Check for XMIN match.
20082007
*/
20092008
if (TransactionIdIsValid(priorXmax) &&
2010-
!TransactionIdEquals(priorXmax, HeapTupleHeaderGetXmin(tp.t_data)))
2009+
!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tp.t_data))
20112010
{
20122011
UnlockReleaseBuffer(buffer);
20132012
break;
@@ -2039,6 +2038,50 @@ heap_get_latest_tid(Relation relation,
20392038
} /* end of loop */
20402039
}
20412040

2041+
/*
2042+
* HeapTupleUpdateXmaxMatchesXmin - verify update chain xmax/xmin lineage
2043+
*
2044+
* Given the new version of a tuple after some update, verify whether the
2045+
* given Xmax (corresponding to the previous version) matches the tuple's
2046+
* Xmin, taking into account that the Xmin might have been frozen after the
2047+
* update.
2048+
*/
2049+
bool
2050+
HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax, HeapTupleHeader htup)
2051+
{
2052+
TransactionId xmin = HeapTupleHeaderGetXmin(htup);
2053+
2054+
/*
2055+
* If the xmax of the old tuple is identical to the xmin of the new one,
2056+
* it's a match.
2057+
*/
2058+
if (TransactionIdEquals(xmax, xmin))
2059+
return true;
2060+
2061+
/*
2062+
* If the Xmin that was in effect prior to a freeze matches the Xmax,
2063+
* it's good too.
2064+
*/
2065+
if (HeapTupleHeaderXminFrozen(htup) &&
2066+
TransactionIdEquals(HeapTupleHeaderGetRawXmin(htup), xmax))
2067+
return true;
2068+
2069+
/*
2070+
* When a tuple is frozen, the original Xmin is lost, but we know it's a
2071+
* committed transaction. So unless the Xmax is InvalidXid, we don't know
2072+
* for certain that there is a match, but there may be one; and we must
2073+
* return true so that a HOT chain that is half-frozen can be walked
2074+
* correctly.
2075+
*
2076+
* We no longer freeze tuples this way, but we must keep this in order to
2077+
* interpret pre-pg_upgrade pages correctly.
2078+
*/
2079+
if (TransactionIdEquals(xmin, FrozenTransactionId) &&
2080+
TransactionIdIsValid(xmax))
2081+
return true;
2082+
2083+
return false;
2084+
}
20422085

20432086
/*
20442087
* UpdateXmaxHintBits - update tuple hint bits after xmax transaction ends
@@ -5400,8 +5443,7 @@ heap_lock_updated_tuple_rec(Relation rel, ItemPointer tid, TransactionId xid,
54005443
* end of the chain, we're done, so return success.
54015444
*/
54025445
if (TransactionIdIsValid(priorXmax) &&
5403-
!TransactionIdEquals(HeapTupleHeaderGetXmin(mytup.t_data),
5404-
priorXmax))
5446+
!HeapTupleUpdateXmaxMatchesXmin(priorXmax, mytup.t_data))
54055447
{
54065448
UnlockReleaseBuffer(buf);
54075449
return HeapTupleMayBeUpdated;

‎src/backend/access/heap/pruneheap.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
465465
* Check the tuple XMIN against prior XMAX, if any
466466
*/
467467
if (TransactionIdIsValid(priorXmax) &&
468-
!TransactionIdEquals(HeapTupleHeaderGetXmin(htup), priorXmax))
468+
!HeapTupleUpdateXmaxMatchesXmin(priorXmax, htup))
469469
break;
470470

471471
/*
@@ -805,7 +805,7 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
805805
htup = (HeapTupleHeader) PageGetItem(page, lp);
806806

807807
if (TransactionIdIsValid(priorXmax) &&
808-
!TransactionIdEquals(priorXmax, HeapTupleHeaderGetXmin(htup)))
808+
!HeapTupleUpdateXmaxMatchesXmin(priorXmax, htup))
809809
break;
810810

811811
/* Remember the root line pointer for this item */

‎src/backend/executor/execMain.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2243,8 +2243,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
22432243
* atomic, and Xmin never changes in an existing tuple, except to
22442244
* invalid or frozen, and neither of those can match priorXmax.)
22452245
*/
2246-
if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
2247-
priorXmax))
2246+
if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tuple.t_data))
22482247
{
22492248
ReleaseBuffer(buffer);
22502249
return NULL;
@@ -2391,8 +2390,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
23912390
/*
23922391
* As above, if xmin isn't what we're expecting, do nothing.
23932392
*/
2394-
if (!TransactionIdEquals(HeapTupleHeaderGetXmin(tuple.t_data),
2395-
priorXmax))
2393+
if (!HeapTupleUpdateXmaxMatchesXmin(priorXmax, tuple.t_data))
23962394
{
23972395
ReleaseBuffer(buffer);
23982396
return NULL;

‎src/include/access/heapam.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ extern void heap_get_latest_tid(Relation relation, Snapshot snapshot,
139139
ItemPointer tid);
140140
extern void setLastTid(const ItemPointer tid);
141141

142+
extern bool HeapTupleUpdateXmaxMatchesXmin(TransactionId xmax,
143+
HeapTupleHeader htup);
144+
142145
extern BulkInsertState GetBulkInsertState(void);
143146
extern void FreeBulkInsertState(BulkInsertState);
144147

0 commit comments

Comments
 (0)
0