Author: Noah Misch Commit: Noah Misch Merge copies of converting an XID to a FullTransactionId. Assume twophase.c is the performance-sensitive caller, and preserve its choice of unlikely() branch hint. Add some retrospective rationale for that choice. Back-patch to v17, for the next commit to use it. Reviewed (in earlier versions) by Michael Paquier. Discussion: https://postgr.es/m/17821-dd8c334263399284@postgresql.org Discussion: https://postgr.es/m/20250116010051.f3.nmisch@google.com diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 8a8e36d..8273123 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1877,7 +1877,9 @@ check_tuple(HeapCheckContext *ctx, bool *xmin_commit_status_ok, /* * Convert a TransactionId into a FullTransactionId using our cached values of * the valid transaction ID range. It is the caller's responsibility to have - * already updated the cached values, if necessary. + * already updated the cached values, if necessary. This is akin to + * FullTransactionIdFromAllowableAt(), but it tolerates corruption in the form + * of an xid before epoch 0. */ static FullTransactionId FullTransactionIdFromXidAndCtx(TransactionId xid, const HeapCheckContext *ctx) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index ab2f4a8..73a8055 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -929,32 +929,16 @@ TwoPhaseGetDummyProc(TransactionId xid, bool lock_held) /* * Compute the FullTransactionId for the given TransactionId. * - * The wrap logic is safe here because the span of active xids cannot exceed one - * epoch at any given time. + * This is safe if the xid has not yet reached COMMIT PREPARED or ROLLBACK + * PREPARED. After those commands, concurrent vac_truncate_clog() may make + * the xid cease to qualify as allowable. XXX Not all callers limit their + * calls accordingly. */ static inline FullTransactionId AdjustToFullTransactionId(TransactionId xid) { - FullTransactionId nextFullXid; - TransactionId nextXid; - uint32 epoch; - Assert(TransactionIdIsValid(xid)); - - LWLockAcquire(XidGenLock, LW_SHARED); - nextFullXid = TransamVariables->nextXid; - LWLockRelease(XidGenLock); - - nextXid = XidFromFullTransactionId(nextFullXid); - epoch = EpochFromFullTransactionId(nextFullXid); - if (unlikely(xid > nextXid)) - { - /* Wraparound occurred, must be from a prev epoch. */ - Assert(epoch > 0); - epoch--; - } - - return FullTransactionIdFromEpochAndXid(epoch, xid); + return FullTransactionIdFromAllowableAt(ReadNextFullTransactionId(), xid); } static inline int diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 3596af0..91b6a91 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -2166,28 +2166,14 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) FullTransactionId XLogRecGetFullXid(XLogReaderState *record) { - TransactionId xid, - next_xid; - uint32 epoch; - /* * This function is only safe during replay, because it depends on the * replay state. See AdvanceNextFullTransactionIdPastXid() for more. */ Assert(AmStartupProcess() || !IsUnderPostmaster); - xid = XLogRecGetXid(record); - next_xid = XidFromFullTransactionId(TransamVariables->nextXid); - epoch = EpochFromFullTransactionId(TransamVariables->nextXid); - - /* - * If xid is numerically greater than next_xid, it has to be from the last - * epoch. - */ - if (unlikely(xid > next_xid)) - --epoch; - - return FullTransactionIdFromEpochAndXid(epoch, xid); + return FullTransactionIdFromAllowableAt(TransamVariables->nextXid, + XLogRecGetXid(record)); } #endif diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index 4736755..20b28b2 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -97,15 +97,11 @@ static bool TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid) { TransactionId xid = XidFromFullTransactionId(fxid); - uint32 now_epoch; - TransactionId now_epoch_next_xid; FullTransactionId now_fullxid; - TransactionId oldest_xid; - FullTransactionId oldest_fxid; + TransactionId oldest_clog_xid; + FullTransactionId oldest_clog_fxid; now_fullxid = ReadNextFullTransactionId(); - now_epoch_next_xid = XidFromFullTransactionId(now_fullxid); - now_epoch = EpochFromFullTransactionId(now_fullxid); if (extracted_xid != NULL) *extracted_xid = xid; @@ -135,52 +131,19 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid) /* * If fxid is not older than TransamVariables->oldestClogXid, the relevant - * CLOG entry is guaranteed to still exist. Convert - * TransamVariables->oldestClogXid into a FullTransactionId to compare it - * with fxid. Determine the right epoch knowing that oldest_fxid - * shouldn't be more than 2^31 older than now_fullxid. + * CLOG entry is guaranteed to still exist. + * + * TransamVariables->oldestXid governs allowable XIDs. Usually, + * oldestClogXid==oldestXid. It's also possible for oldestClogXid to + * follow oldestXid, in which case oldestXid might advance after our + * ReadNextFullTransactionId() call. If oldestXid has advanced, that + * advancement reinstated the usual oldestClogXid==oldestXid. Whether or + * not that happened, oldestClogXid is allowable relative to now_fullxid. */ - oldest_xid = TransamVariables->oldestClogXid; - Assert(TransactionIdPrecedesOrEquals(oldest_xid, now_epoch_next_xid)); - if (oldest_xid <= now_epoch_next_xid) - { - oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch, oldest_xid); - } - else - { - Assert(now_epoch > 0); - oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch - 1, oldest_xid); - } - return !FullTransactionIdPrecedes(fxid, oldest_fxid); -} - -/* - * Convert a TransactionId obtained from a snapshot held by the caller to a - * FullTransactionId. Use next_fxid as a reference FullTransactionId, so that - * we can compute the high order bits. It must have been obtained by the - * caller with ReadNextFullTransactionId() after the snapshot was created. - */ -static FullTransactionId -widen_snapshot_xid(TransactionId xid, FullTransactionId next_fxid) -{ - TransactionId next_xid = XidFromFullTransactionId(next_fxid); - uint32 epoch = EpochFromFullTransactionId(next_fxid); - - /* Special transaction ID. */ - if (!TransactionIdIsNormal(xid)) - return FullTransactionIdFromEpochAndXid(0, xid); - - /* - * The 64 bit result must be <= next_fxid, since next_fxid hadn't been - * issued yet when the snapshot was created. Every TransactionId in the - * snapshot must therefore be from the same epoch as next_fxid, or the - * epoch before. We know this because next_fxid is never allow to get - * more than one epoch ahead of the TransactionIds in any snapshot. - */ - if (xid > next_xid) - epoch--; - - return FullTransactionIdFromEpochAndXid(epoch, xid); + oldest_clog_xid = TransamVariables->oldestClogXid; + oldest_clog_fxid = + FullTransactionIdFromAllowableAt(now_fullxid, oldest_clog_xid); + return !FullTransactionIdPrecedes(fxid, oldest_clog_fxid); } /* @@ -420,12 +383,18 @@ pg_current_snapshot(PG_FUNCTION_ARGS) nxip = cur->xcnt; snap = palloc(PG_SNAPSHOT_SIZE(nxip)); - /* fill */ - snap->xmin = widen_snapshot_xid(cur->xmin, next_fxid); - snap->xmax = widen_snapshot_xid(cur->xmax, next_fxid); + /* + * Fill. This is the current backend's active snapshot, so MyProc->xmin + * is <= all these XIDs. As long as that remains so, oldestXid can't + * advance past any of these XIDs. Hence, these XIDs remain allowable + * relative to next_fxid. + */ + snap->xmin = FullTransactionIdFromAllowableAt(next_fxid, cur->xmin); + snap->xmax = FullTransactionIdFromAllowableAt(next_fxid, cur->xmax); snap->nxip = nxip; for (i = 0; i < nxip; i++) - snap->xip[i] = widen_snapshot_xid(cur->xip[i], next_fxid); + snap->xip[i] = + FullTransactionIdFromAllowableAt(next_fxid, cur->xip[i]); /* * We want them guaranteed to be in ascending order. This also removes diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 0cab865..7d82cd2 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -370,6 +370,49 @@ FullTransactionIdNewer(FullTransactionId a, FullTransactionId b) return b; } +/* + * Compute FullTransactionId for the given TransactionId, assuming xid was + * between [oldestXid, nextXid] at the time when TransamVariables->nextXid was + * nextFullXid. When adding calls, evaluate what prevents xid from preceding + * oldestXid if SetTransactionIdLimit() runs between the collection of xid and + * the collection of nextFullXid. + */ +static inline FullTransactionId +FullTransactionIdFromAllowableAt(FullTransactionId nextFullXid, + TransactionId xid) +{ + uint32 epoch; + + /* Special transaction ID. */ + if (!TransactionIdIsNormal(xid)) + return FullTransactionIdFromEpochAndXid(0, xid); + + Assert(TransactionIdPrecedesOrEquals(xid, + XidFromFullTransactionId(nextFullXid))); + + /* + * The 64 bit result must be <= nextFullXid, since nextFullXid hadn't been + * issued yet when xid was in the past. The xid must therefore be from + * the epoch of nextFullXid or the epoch before. We know this because we + * must remove (by freezing) an XID before assigning the XID half an epoch + * ahead of it. + * + * The unlikely() branch hint is dubious. It's perfect for the first 2^32 + * XIDs of a cluster's life. Right at 2^32 XIDs, misprediction shoots to + * 100%, then improves until perfection returns 2^31 XIDs later. Since + * current callers pass relatively-recent XIDs, expect >90% prediction + * accuracy overall. This favors average latency over tail latency. + */ + epoch = EpochFromFullTransactionId(nextFullXid); + if (unlikely(xid > XidFromFullTransactionId(nextFullXid))) + { + Assert(epoch != 0); + epoch--; + } + + return FullTransactionIdFromEpochAndXid(epoch, xid); +} + #endif /* FRONTEND */ #endif /* TRANSAM_H */