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[email protected]

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 */