From d44060cfcc49d512da1ae9b1b846385748e46d04 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Thu, 8 Feb 2024 12:45:26 +0200 Subject: [PATCH] Fix wrong logic in TransactionIdInRecentPast() The TransactionIdInRecentPast() should return false for all the transactions older than TransamVariables->oldestClogXid. However, the function contains a bug in comparison FullTransactionId to TransactionID allowing full transactions between nextXid - 2^32 and oldestClogXid - 2^31. This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into FullTransactionId first, then performing the comparison. Backpatch to all supported versions. Reported-by: Egor Chindyaskin Bug: 18212 Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org Author: Karina Litskevich Reviewed-by: Kyotaro Horiguchi Backpatch-through: 12 --- src/backend/utils/adt/txid.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c index de5eeb530c..a2343b9a53 100644 --- a/src/backend/utils/adt/txid.c +++ b/src/backend/utils/adt/txid.c @@ -116,10 +116,14 @@ TransactionIdInRecentPast(uint64 xid_with_epoch, TransactionId *extracted_xid) { uint32 xid_epoch = (uint32) (xid_with_epoch >> 32); TransactionId xid = (TransactionId) xid_with_epoch; + FullTransactionId fxid; uint32 now_epoch; TransactionId now_epoch_next_xid; FullTransactionId now_fullxid; + TransactionId oldest_xid; + FullTransactionId oldest_fxid; + fxid = FullTransactionIdFromEpochAndXid(xid_epoch, xid); now_fullxid = ReadNextFullTransactionId(); now_epoch_next_xid = XidFromFullTransactionId(now_fullxid); now_epoch = EpochFromFullTransactionId(now_fullxid); @@ -151,17 +155,24 @@ TransactionIdInRecentPast(uint64 xid_with_epoch, TransactionId *extracted_xid) Assert(LWLockHeldByMe(CLogTruncationLock)); /* - * If the transaction ID has wrapped around, it's definitely too old to - * determine the commit status. Otherwise, we can compare it to - * ShmemVariableCache->oldestClogXid to determine whether the relevant - * CLOG entry is guaranteed to still exist. + * If fxid is not older than ShmemVariableCache->oldestClogXid, the + * relevant CLOG entry is guaranteed to still exist. Convert + * ShmemVariableCache->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. */ - if (xid_epoch + 1 < now_epoch - || (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid) - || TransactionIdPrecedes(xid, ShmemVariableCache->oldestClogXid)) - return false; - - return true; + oldest_xid = ShmemVariableCache->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); } /*