diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 85533e12a1..bad2a89e4f 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -6208,10 +6208,10 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, * been pruned away instead, since updater XID is < OldestXmin). * Just remove xmax. */ - if (TransactionIdDidCommit(update_xact)) + if (!TransactionIdDidAbort(update_xact)) ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED), - errmsg_internal("multixact %u contains committed update XID %u from before removable cutoff %u", + errmsg_internal("multixact %u contains non-aborted update XID %u from before removable cutoff %u", multi, update_xact, cutoffs->OldestXmin))); *flags |= FRM_INVALIDATE_XMAX; @@ -6500,10 +6500,11 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, freeze_xmax = false; TransactionId xid; - frz->frzflags = 0; + frz->xmax = HeapTupleHeaderGetRawXmax(tuple); frz->t_infomask2 = tuple->t_infomask2; frz->t_infomask = tuple->t_infomask; - frz->xmax = HeapTupleHeaderGetRawXmax(tuple); + frz->frzflags = 0; + frz->checkflags = 0; /* * Process xmin, while keeping track of whether it's already frozen, or @@ -6521,14 +6522,12 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, errmsg_internal("found xmin %u from before relfrozenxid %u", xid, cutoffs->relfrozenxid))); - freeze_xmin = TransactionIdPrecedes(xid, cutoffs->OldestXmin); - if (freeze_xmin && !TransactionIdDidCommit(xid)) - ereport(ERROR, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg_internal("uncommitted xmin %u from before xid cutoff %u needs to be frozen", - xid, cutoffs->OldestXmin))); - /* Will set freeze_xmin flags in freeze plan below */ + freeze_xmin = TransactionIdPrecedes(xid, cutoffs->OldestXmin); + + /* Verify that xmin committed if and when freeze plan is executed */ + if (freeze_xmin) + frz->checkflags |= HEAP_FREEZE_CHECK_XMIN_COMMITTED; } /* @@ -6551,7 +6550,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, } /* Now process xmax */ - xid = HeapTupleHeaderGetRawXmax(tuple); + xid = frz->xmax; if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) { /* Raw xmax is a MultiXactId */ @@ -6662,21 +6661,16 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, errmsg_internal("found xmax %u from before relfrozenxid %u", xid, cutoffs->relfrozenxid))); - if (TransactionIdPrecedes(xid, cutoffs->OldestXmin)) - freeze_xmax = true; + /* Will set freeze_xmax flags in freeze plan below */ + freeze_xmax = TransactionIdPrecedes(xid, cutoffs->OldestXmin); /* - * If we freeze xmax, make absolutely sure that it's not an XID that - * is important. (Note, a lock-only xmax can be removed independent - * of committedness, since a committed lock holder has released the - * lock). + * Verify that xmax aborted if and when freeze plan is executed, + * provided it's from an update. (A lock-only xmax can be removed + * independent of this, since the lock is released at xact end.) */ - if (freeze_xmax && !HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask) && - TransactionIdDidCommit(xid)) - ereport(ERROR, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg_internal("cannot freeze committed xmax %u", - xid))); + if (freeze_xmax && !HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) + frz->checkflags |= HEAP_FREEZE_CHECK_XMAX_ABORTED; } else if (!TransactionIdIsValid(xid)) { @@ -6804,19 +6798,60 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer, Assert(ntuples > 0); - START_CRIT_SECTION(); + /* + * Perform xmin/xmax XID status sanity checks before critical section. + * + * heap_prepare_freeze_tuple doesn't perform these checks directly because + * pg_xact lookups are relatively expensive. They shouldn't be repeated + * by successive VACUUMs that each decide against freezing the same page. + */ + for (int i = 0; i < ntuples; i++) + { + HeapTupleFreeze *frz = tuples + i; + ItemId itemid = PageGetItemId(page, frz->offset); + HeapTupleHeader htup; - MarkBufferDirty(buffer); + htup = (HeapTupleHeader) PageGetItem(page, itemid); + + /* Deliberately avoid relying on tuple hint bits here */ + if (frz->checkflags & HEAP_FREEZE_CHECK_XMIN_COMMITTED) + { + TransactionId xmin = HeapTupleHeaderGetRawXmin(htup); + + Assert(!HeapTupleHeaderXminFrozen(htup)); + if (unlikely(!TransactionIdDidCommit(xmin))) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg_internal("uncommitted xmin %u needs to be frozen", + xmin))); + } + if (frz->checkflags & HEAP_FREEZE_CHECK_XMAX_ABORTED) + { + TransactionId xmax = HeapTupleHeaderGetRawXmax(htup); + + Assert(TransactionIdIsNormal(xmax)); + if (unlikely(!TransactionIdDidAbort(xmax))) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg_internal("cannot freeze non-aborted xmax %u", + xmax))); + } + } + + START_CRIT_SECTION(); for (int i = 0; i < ntuples; i++) { + HeapTupleFreeze *frz = tuples + i; + ItemId itemid = PageGetItemId(page, frz->offset); HeapTupleHeader htup; - ItemId itemid = PageGetItemId(page, tuples[i].offset); htup = (HeapTupleHeader) PageGetItem(page, itemid); - heap_execute_freeze_tuple(htup, &tuples[i]); + heap_execute_freeze_tuple(htup, frz); } + MarkBufferDirty(buffer); + /* Now WAL-log freezing if necessary */ if (RelationNeedsWAL(rel)) { diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 4a69b6dd04..417108f1e0 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -100,6 +100,13 @@ typedef enum HEAPTUPLE_DELETE_IN_PROGRESS /* deleting xact is still in progress */ } HTSV_Result; +/* + * heap_prepare_freeze_tuple may request that heap_freeze_execute_prepared + * check any tuple's to-be-frozen xmin and/or xmax status using pg_xact + */ +#define HEAP_FREEZE_CHECK_XMIN_COMMITTED 0x01 +#define HEAP_FREEZE_CHECK_XMAX_ABORTED 0x02 + /* heap_prepare_freeze_tuple state describing how to freeze a tuple */ typedef struct HeapTupleFreeze { @@ -109,6 +116,8 @@ typedef struct HeapTupleFreeze uint16 t_infomask; uint8 frzflags; + /* xmin/xmax check flags */ + uint8 checkflags; /* Page offset number for tuple */ OffsetNumber offset; } HeapTupleFreeze;