0001-WIP-Fix-corruption-due-to-vacuum_defer_cleanup_age-u.patch

text/plain

Filename: 0001-WIP-Fix-corruption-due-to-vacuum_defer_cleanup_age-u.patch
Type: text/plain
Part: 0
Message: Re: BUG: Postgres 14 + vacuum_defer_cleanup_age + FOR UPDATE + UPDATE

Patch

Format: format-patch
Series: patch 0001
Subject: WIP: Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids
File+
src/backend/storage/ipc/procarray.c 22 34
src/include/access/transam.h 65 6
From ffa80fece3384e3a12a52de18102a0d169f52841 Mon Sep 17 00:00:00 2001
From: Nkey <michail.nikolaev@gmail.com>
Date: Sat, 4 Feb 2023 15:16:52 +0300
Subject: [PATCH] WIP: Fix corruption due to vacuum_defer_cleanup_age
 underflowing 64bit xids

---
 src/backend/storage/ipc/procarray.c | 56 ++++++++++++-----------------
 src/include/access/transam.h        | 71 +++++++++++++++++++++++++++++++++----
 2 files changed, 87 insertions(+), 40 deletions(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 4340bf9641..5dd662cd75 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -368,8 +368,6 @@ static void ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid);
 static void MaintainLatestCompletedXid(TransactionId latestXid);
 static void MaintainLatestCompletedXidRecovery(TransactionId latestXid);
 
-static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
-												  TransactionId xid);
 static void GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons);
 
 /*
@@ -1889,16 +1887,32 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
 		 * in varsup.c.  Also note that we intentionally don't apply
 		 * vacuum_defer_cleanup_age on standby servers.
 		 */
+		Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+											 h->shared_oldest_nonremovable));
+		Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+											 h->data_oldest_nonremovable));
+
 		h->oldest_considered_running =
 			TransactionIdRetreatedBy(h->oldest_considered_running,
-									 vacuum_defer_cleanup_age);
-		h->shared_oldest_nonremovable =
+									 vacuum_defer_cleanup_age,
+									 h->latest_completed);
+
+		h->shared_oldest_nonremovable = 
 			TransactionIdRetreatedBy(h->shared_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
-		h->data_oldest_nonremovable =
+									 vacuum_defer_cleanup_age,
+									 h->latest_completed);
+
+		h->data_oldest_nonremovable = 
 			TransactionIdRetreatedBy(h->data_oldest_nonremovable,
-									 vacuum_defer_cleanup_age);
+									 vacuum_defer_cleanup_age,
+									h->latest_completed);
+
 		/* defer doesn't apply to temp relations */
+
+		Assert(TransactionIdPrecedesOrEquals(h->oldest_considered_running,
+											 h->shared_oldest_nonremovable));
+		Assert(TransactionIdPrecedesOrEquals(h->shared_oldest_nonremovable,
+											 h->data_oldest_nonremovable));
 	}
 
 	/*
@@ -2471,7 +2485,7 @@ GetSnapshotData(Snapshot snapshot)
 
 		/* apply vacuum_defer_cleanup_age */
 		def_vis_xid_data =
-			TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age);
+			TransactionIdRetreatedBy(xmin, vacuum_defer_cleanup_age, oldestfxid);
 
 		/* Check whether there's a replication slot requiring an older xmin. */
 		def_vis_xid_data =
@@ -4295,32 +4309,6 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
 	return GlobalVisTestIsRemovableXid(state, xid);
 }
 
-/*
- * Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
- * is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).
- *
- * Be very careful about when to use this function. It can only safely be used
- * when there is a guarantee that xid is within MaxTransactionId / 2 xids of
- * rel. That e.g. can be guaranteed if the caller assures a snapshot is
- * held by the backend and xid is from a table (where vacuum/freezing ensures
- * the xid has to be within that range), or if xid is from the procarray and
- * prevents xid wraparound that way.
- */
-static inline FullTransactionId
-FullXidRelativeTo(FullTransactionId rel, TransactionId xid)
-{
-	TransactionId rel_xid = XidFromFullTransactionId(rel);
-
-	Assert(TransactionIdIsValid(xid));
-	Assert(TransactionIdIsValid(rel_xid));
-
-	/* not guaranteed to find issues, but likely to catch mistakes */
-	AssertTransactionIdInAllowableRange(xid);
-
-	return FullTransactionIdFromU64(U64FromFullTransactionId(rel)
-									+ (int32) (xid - rel_xid));
-}
-
 
 /* ----------------------------------------------
  *		KnownAssignedTransactionIds sub-module
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index f5af6d3055..e094e80859 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -317,16 +317,75 @@ ReadNextTransactionId(void)
 	return XidFromFullTransactionId(ReadNextFullTransactionId());
 }
 
-/* return transaction ID backed up by amount, handling wraparound correctly */
+/*
+ * Convert a 32 bit transaction id into 64 bit transaction id, by assuming it
+ * is within MaxTransactionId / 2 of XidFromFullTransactionId(rel).
+ *
+ * Be very careful about when to use this function. It can only safely be used
+ * when there is a guarantee that xid is within MaxTransactionId / 2 xids of
+ * rel. That e.g. can be guaranteed if the caller assures a snapshot is
+ * held by the backend and xid is from a table (where vacuum/freezing ensures
+ * the xid has to be within that range), or if xid is from the procarray and
+ * prevents xid wraparound that way.
+ */
+static inline FullTransactionId
+FullXidRelativeTo(FullTransactionId rel, TransactionId xid)
+{
+	TransactionId rel_xid = XidFromFullTransactionId(rel);
+
+	Assert(TransactionIdIsValid(xid));
+	Assert(TransactionIdIsValid(rel_xid));
+
+	/* not guaranteed to find issues, but likely to catch mistakes */
+	AssertTransactionIdInAllowableRange(xid);
+
+	return FullTransactionIdFromU64(U64FromFullTransactionId(rel)
+		+ (int32)(xid - rel_xid));
+}
+
+/*
+ * return transaction ID backed up by amount, handling wraparound correctly
+ *
+ * Need to be careful to prevent xid from retreating below
+ * FirstNormalTransactionId during epoch 0. This is important to prevent
+ * generating xids that cannot be converted to a FullTransactionId without
+ * wrapping around.
+ *
+ * If amount would lead to a too old xid, FirstNormalTransactionId is
+ * returned instead.
+ */
+
 static inline TransactionId
-TransactionIdRetreatedBy(TransactionId xid, uint32 amount)
+TransactionIdRetreatedBy(TransactionId xid, int amount, FullTransactionId rel)
 {
-	xid -= amount;
+	FullTransactionId fxid;
+	uint64		fxid_i;
+	TransactionId r;
+
+	Assert(TransactionIdIsNormal(xid));
+	Assert(amount >= 0);	/* relevant GUCs are stored as ints */
+	AssertTransactionIdInAllowableRange(xid);
+
+	if (amount == 0)
+		return xid;
+
+	fxid = FullXidRelativeTo(rel, xid);
+	fxid_i = U64FromFullTransactionId(fxid);
+
+	if ((fxid_i - FirstNormalTransactionId) <= amount)
+		r = FirstNormalTransactionId;
+	else
+	{
+		r = xid - amount;
+
+		while (r < FirstNormalTransactionId)
+			r--;
 
-	while (xid < FirstNormalTransactionId)
-		xid--;
+		Assert(TransactionIdIsNormal(r));
+		Assert(NormalTransactionIdPrecedes(r, xid));
+	}
 
-	return xid;
+	return r;
 }
 
 /* return the older of the two IDs */
-- 
2.16.2.windows.1