From 37f3cf2906308b00b31367cbaef90b7fc23759e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Rito?= Date: Tue, 10 Nov 2026 17:31:18 -0300 Subject: [PATCH v2] xid_wraparound: Fix consume_xids() consuming one XID too many MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit consume_xids() and consume_xids_until() called GetTopTransactionId() without counting the XID that this may assign, so when the transaction had no XID yet, they consumed one more XID than requested and returned a XID that was not the last one they assigned. Count the XID obtained here and remember it in lastxid, so that the result is correct even for consume_xids(1). Since consume_xids(0) cannot return an XID assigned by the function, make it raise an error instead of returning the next XID. Also, replace the hard-coded shortcut threshold of 2000 with XID_SHORTCUT_THRESHOLD, set to 4000 so that the shortcut cannot skip more XIDs than requested even with 32kB blocks, where XidSkip() can return up to 3276. Add an assertion to XidSkip() to verify this. Document what the two functions return. Author: Yushi Ogiwara Author: João Rito Reviewed-by: Masahiko Sawada Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/2a62f138c9dcb6fcf5889b076d123aa2@oss.nttdata.com --- .../modules/xid_wraparound/xid_wraparound.c | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/src/test/modules/xid_wraparound/xid_wraparound.c b/src/test/modules/xid_wraparound/xid_wraparound.c index ca25d7e020..3de3ff472e 100644 --- a/src/test/modules/xid_wraparound/xid_wraparound.c +++ b/src/test/modules/xid_wraparound/xid_wraparound.c @@ -26,6 +26,8 @@ static FullTransactionId consume_xids_common(FullTransactionId untilxid, uint64 /* * Consume the specified number of XIDs. + * + * Returns the last XID assigned by this function. */ PG_FUNCTION_INFO_V1(consume_xids); Datum @@ -34,19 +36,18 @@ consume_xids(PG_FUNCTION_ARGS) int64 nxids = PG_GETARG_INT64(0); FullTransactionId lastxid; - if (nxids < 0) + if (nxids <= 0) elog(ERROR, "invalid nxids argument: %" PRId64, nxids); - if (nxids == 0) - lastxid = ReadNextFullTransactionId(); - else - lastxid = consume_xids_common(InvalidFullTransactionId, (uint64) nxids); + lastxid = consume_xids_common(InvalidFullTransactionId, (uint64) nxids); PG_RETURN_FULLTRANSACTIONID(lastxid); } /* * Consume XIDs, up to the given XID. + * + * Returns the last XID assigned by this function. */ PG_FUNCTION_INFO_V1(consume_xids_until); Datum @@ -64,6 +65,23 @@ consume_xids_until(PG_FUNCTION_ARGS) PG_RETURN_FULLTRANSACTIONID(lastxid); } +/* + * These constants copied from .c files, because they're private. + */ +#define COMMIT_TS_XACTS_PER_PAGE (BLCKSZ / 10) +#define SUBTRANS_XACTS_PER_PAGE (BLCKSZ / sizeof(TransactionId)) +#define CLOG_XACTS_PER_BYTE 4 +#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE) + +/* + * Take the shortcut in consume_xids_common() only if we have more than this + * many XIDs left to consume. XidSkip() never skips more than + * COMMIT_TS_XACTS_PER_PAGE XIDs, which is 3276 with the largest supported + * block size (32kB), so this is enough to make sure that the shortcut never + * consumes more XIDs than requested. + */ +#define XID_SHORTCUT_THRESHOLD 4000 + /* * Common functionality between the two public functions. */ @@ -89,7 +107,11 @@ consume_xids_common(FullTransactionId untilxid, uint64 nxids) * the cache overflows, but beyond that, we don't keep track of the * consumed XIDs. */ - (void) GetTopTransactionId(); + if (!FullTransactionIdIsValid(GetTopFullTransactionIdIfAny())) + { + lastxid = GetTopFullTransactionId(); + consumed++; + } for (;;) { @@ -115,7 +137,7 @@ consume_xids_common(FullTransactionId untilxid, uint64 nxids) * If we still have plenty of XIDs to consume, try to take a shortcut * and bump up the nextXid counter directly. */ - if (xids_left > 2000 && + if (xids_left > XID_SHORTCUT_THRESHOLD && consumed - last_reported_at < REPORT_INTERVAL && MyProc->subxidStatus.overflowed) { @@ -153,14 +175,6 @@ consume_xids_common(FullTransactionId untilxid, uint64 nxids) return lastxid; } -/* - * These constants copied from .c files, because they're private. - */ -#define COMMIT_TS_XACTS_PER_PAGE (BLCKSZ / 10) -#define SUBTRANS_XACTS_PER_PAGE (BLCKSZ / sizeof(TransactionId)) -#define CLOG_XACTS_PER_BYTE 4 -#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE) - /* * All the interesting action in GetNewTransactionId happens when we extend * the SLRUs, or at the uint32 wraparound. If the nextXid counter is not close @@ -193,6 +207,9 @@ XidSkip(FullTransactionId fullxid) return 0; distance = Min(distance, CLOG_XACTS_PER_PAGE - rem); + /* The shortcut threshold in consume_xids_common() relies on this */ + Assert(distance <= XID_SHORTCUT_THRESHOLD); + return distance; } -- 2.53.0.windows.1