On 17/09/2026 19:21, Noah Misch wrote:
On Thu, Sep 17, 2026 at 06:25:58PM +0300, Heikki Linnakangas wrote:
On 16/09/2026 14:33, Heikki Linnakangas wrote:
On 28/08/2026 02:17, Noah Misch wrote:
commit bd8d9c9 wrote:
Commit:     Heikki Linnakangas <[email protected]>
CommitDate: Tue Dec 9 13:53:03 2025 +0200

      Widen MultiXactOffset to 64 bits

--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c

+        nxtmulti = old_cluster.controldata.chkpnt_nxtmulti;
+        if (old_cluster.controldata.cat_ver >=
MULTIXACT_FORMATCHANGE_CAT_VER)
+        {
+            /* Versions 9.3 - 18: convert all multixids  */
+            oldstMulti = old_cluster.controldata.chkpnt_oldstMulti;

If a cluster's upgrade history includes an upgrade from 9.3 to early
9.4, it
may have a wrong value here.  Specifically, upgrades done before a61daa14
(2014-07 commit) have that hazard.  We still have backend code to
detect such
cases and reduce damage:

         ereport(LOG,
                 (errmsg("cannot truncate up to MultiXact %u because
it does not exist on disk, skipping truncation",
                         newOldestMulti)));

However, the pg_upgrade side from the v19 commit lacks such
protection.  If
heap tuples still reference older multixacts than the faulty control data
suggests, pg_upgrade will copy too small a range, making affected tuples
unreadable.

Thanks, I'll look into this. My first reaction is that I think if
oldstMulti is incorrectly too old, the upgrade will fail because the
conversion routine will fail to find it. If it's too new, i.e. "in the
future", it will also fail to find it.

Agreed, those cases fail cleanly.  No concerns on those.

The third possibility is that the bogus oldstMulti value is within the
range of the "real" range.

That's the only scenario I felt was at risk.  For example, the range of MXIDs
in tuple headers is [(unsigned)-100M,200M], but oldestMulti is 1.

That can happen if multixid wraparound had
already happened before the (broken) 9.3 -> 9.4 upgrade. In that case,
even if the multixids are still readable on disk, you're one vacuum away
from truncating them. In other words, the damage has already been done,
or could be done at any minute.

Okay.  I briefly tried to verify how rapidly one can expect v18 to truncate
the SLRUs in this scenario, but I didn't get very far.  I will rely on your
analysis.  Thanks for studying it.

Thinking about this some more, you were right originally, there is danger here:

If you're in that situation, where oldestMulti is higher than min(datminmxid), VACUUM will _not_ truncate the slrus. That was the whole idea of commit 78db307bb2, to refrain from truncating. So you're not in much danger, unless you consume so many multixids that you start to overwrite the tail.

But v19 pg_upgrade would indeed effectively truncate them away, by not converting them to the new format.


A better fix here is to add a pre-check that oldestMulti < min(datminmxid). That seems like a good sanity check anyway, and it will catch that ancient 9.3 scenario.

Barring objections, I'll commit the attached to add that check. I didn't include a hint about that 9.3 upgrade bug, since I don't think you're very likely to hit that anymore. But it's good to have the check to prevent damage if you do manage to hit it, or some other bug.

- Heikki
From 866999c1251fbbde2f1d9db2799734c7c6e8fe54 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Fri, 18 Sep 2026 20:57:26 +0300
Subject: [PATCH 1/1] Check that oldestXID and oldestMulti are consistent at
 pg_upgrade

Now that pg_upgrade will rewrite multixid members, starting from
oldestMulti, it's important that oldestMulti is valid.  Add a sanity
check that oldestMulti is not newer than the oldest datminmxid value
in pg_database.

One case where this could happen is if the cluster was previously
upgraded to version 9.3 with a buggy pg_upgrade version that didn't
have commit a61daa14d5.  This new pg_upgrade check is similar to the
defence that was added in commit 78db307bb2 to VACUUM to avoid
truncating away multixids if oldestMulti is too new.  This pg_upgrade
check differs in that we don't try to soldier on with the upgrade if
the oldestMultiXID is inconsistent, but rather just abort the upgrade.

Reported-by: Noah Misch <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
Backpatch-through: 19
---
 src/backend/access/transam/multixact.c | 28 -----------
 src/bin/pg_upgrade/check.c             | 68 ++++++++++++++++++++++++++
 src/include/access/multixact.h         | 32 ++++++++++--
 3 files changed, 97 insertions(+), 31 deletions(-)

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 70a4ea69486..3caedb54850 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -2860,34 +2860,6 @@ MultiXactMemberIoErrorDetail(const void *opaque_data)
 						 context->offset);
 }
 
-/*
- * Decide which of two MultiXactIds is earlier.
- *
- * XXX do we need to do something special for InvalidMultiXactId?
- * (Doesn't look like it.)
- */
-bool
-MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2)
-{
-	int32		diff = (int32) (multi1 - multi2);
-
-	return (diff < 0);
-}
-
-/*
- * MultiXactIdPrecedesOrEquals -- is multi1 logically <= multi2?
- *
- * XXX do we need to do something special for InvalidMultiXactId?
- * (Doesn't look like it.)
- */
-bool
-MultiXactIdPrecedesOrEquals(MultiXactId multi1, MultiXactId multi2)
-{
-	int32		diff = (int32) (multi1 - multi2);
-
-	return (diff <= 0);
-}
-
 
 /*
  * Write a TRUNCATE xlog record
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 41342561763..758fb8fb62b 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -9,6 +9,8 @@
 
 #include "postgres_fe.h"
 
+#include "access/multixact.h"
+#include "access/transam.h"
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_authid_d.h"
 #include "catalog/pg_class_d.h"
@@ -36,6 +38,7 @@ static void check_new_cluster_subscription_configuration(void);
 static void check_old_cluster_for_valid_slots(void);
 static void check_old_cluster_subscription_state(void);
 static void check_old_cluster_global_names(ClusterInfo *cluster);
+static void check_for_oldestXid_consistency(ClusterInfo *cluster);
 
 /*
  * DataTypesUsageChecks - definitions of data type checks for the old cluster
@@ -570,6 +573,7 @@ check_and_dump_old_cluster(void)
 	 */
 	check_is_install_user(&old_cluster);
 	check_for_prepared_transactions(&old_cluster);
+	check_for_oldestXid_consistency(&old_cluster);
 	check_for_isn_and_int8_passing_mismatch(&old_cluster);
 
 	if (GET_MAJOR_VERSION(old_cluster.major_version) >= 1700)
@@ -2570,3 +2574,67 @@ check_old_cluster_global_names(ClusterInfo *cluster)
 	else
 		check_ok();
 }
+
+/*
+ * check_for_oldestXid_consistency()
+ *
+ * Check that the oldestXID and oldestMultiXID values in the control file are
+ * consistent with the 'datfrozenxid' and 'datminmxid' values in pg_database.
+ *
+ * The invariant is that value in the control file must always be equal or
+ * older than the oldest datfrozenxid.  Otherwise you might already have
+ * truncated away clog or multixids that are still needed.  If that has
+ * happened, we refuse the upgrade and require the administrator to deal with
+ * the situation first.
+ *
+ * One scenario where that is known to happen is if the cluster was upgraded
+ * in the past to version 9.3 with a buggy pg_upgrade version that didn't copy
+ * the oldestMulti value from the old cluster.  See commit a61daa14d5.  That
+ * was a long time ago, though, so you're not very likely to encounter that
+ * bug in the wild anymore.  Therefore we don't assume that's the cause or try
+ * to do anything clever here. In any case, it's still good to check to
+ * prevent further damage.
+ */
+static void
+check_for_oldestXid_consistency(ClusterInfo *cluster)
+{
+	PGconn	   *conn_template1;
+	PGresult   *dbres;
+	int			ntups;
+	int			i_datname;
+	int			i_datfrozenxid;
+	int			i_datminmxid;
+
+	prep_status("Checking oldestXID and oldestMultiXid consistency");
+
+	conn_template1 = connectToServer(cluster, "template1");
+
+	dbres = executeQueryOrDie(conn_template1,
+							  "SELECT datname, datfrozenxid, datminmxid "
+							  "FROM	pg_catalog.pg_database");
+
+	i_datname = PQfnumber(dbres, "datname");
+	i_datfrozenxid = PQfnumber(dbres, "datfrozenxid");
+	i_datminmxid = PQfnumber(dbres, "datminmxid");
+
+	ntups = PQntuples(dbres);
+	for (int dbnum = 0; dbnum < ntups; dbnum++)
+	{
+		char	   *datname = PQgetvalue(dbres, dbnum, i_datname);
+		TransactionId datfrozenxid = (TransactionId) str2uint(PQgetvalue(dbres, dbnum, i_datfrozenxid));
+		MultiXactId datminmxid = (MultiXactId) str2uint(PQgetvalue(dbres, dbnum, i_datminmxid));
+
+		if (TransactionIdPrecedes(datfrozenxid, cluster->controldata.chkpnt_oldstxid))
+		{
+			pg_fatal("oldestXID (%u) in the control file is newer than the datfrozenxid (%u) of database \"%s\"",
+					 cluster->controldata.chkpnt_oldstxid, datfrozenxid, datname);
+		}
+		if (MultiXactIdPrecedes(datminmxid, cluster->controldata.chkpnt_oldstMulti))
+		{
+			pg_fatal("oldestMultiXid (%u) in control file is newer than the datminmxid (%u) of database \"%s\"",
+					 cluster->controldata.chkpnt_oldstMulti, datminmxid, datname);
+		}
+	}
+
+	check_ok();
+}
diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h
index 6be5299ab68..503ec327404 100644
--- a/src/include/access/multixact.h
+++ b/src/include/access/multixact.h
@@ -110,9 +110,35 @@ extern int	GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
 extern void GetMultiXactInfo(uint32 *multixacts, MultiXactOffset *nextOffset,
 							 MultiXactId *oldestMultiXactId,
 							 MultiXactOffset *oldestOffset);
-extern bool MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2);
-extern bool MultiXactIdPrecedesOrEquals(MultiXactId multi1,
-										MultiXactId multi2);
+
+
+/*
+ * Decide which of two MultiXactIds is earlier.
+ *
+ * XXX do we need to do something special for InvalidMultiXactId?
+ * (Doesn't look like it.)
+ */
+static inline bool
+MultiXactIdPrecedes(MultiXactId multi1, MultiXactId multi2)
+{
+	int32		diff = (int32) (multi1 - multi2);
+
+	return (diff < 0);
+}
+
+/*
+ * MultiXactIdPrecedesOrEquals -- is multi1 logically <= multi2?
+ *
+ * XXX do we need to do something special for InvalidMultiXactId?
+ * (Doesn't look like it.)
+ */
+static inline bool
+MultiXactIdPrecedesOrEquals(MultiXactId multi1, MultiXactId multi2)
+{
+	int32		diff = (int32) (multi1 - multi2);
+
+	return (diff <= 0);
+}
 
 extern int	multixactoffsetssyncfiletag(const FileTag *ftag, char *path);
 extern int	multixactmemberssyncfiletag(const FileTag *ftag, char *path);
-- 
2.47.3

Reply via email to