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 8a8e36dde7e4b98fca4e3b5c06dad47862fa8c21..827312306f6043ff98716e6ee59c200d71dfae40 100644 (file)
--- 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 ab2f4a8a92fd292abc678c24809470a94502946c..73a80559194e75f5ca2c8c69060b35b79199ff3f 100644 (file)
--- 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 3596af06172ec6c05dce022ae76995db83cdafd0..91b6a91767d6b5acd735af143561cdaebe8205a1 100644 (file)
--- 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 4736755b29853d18b8291d0bdb47dbbf0234ce45..20b28b2528c8f2df96b41f84aca28912beb2fd87 100644 (file)
--- 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.
-        */
-       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.
+        * 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.
         */
-       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 0cab8653f1b65ab1ed76799d9c2862d3b391fabe..7d82cd2eb5621ef57e7e232ff95dabf5e44d0b44 100644 (file)
--- 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 */