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.

The third possibility is that the bogus oldstMulti value is within the range of the "real" range. 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. That's the scenario for which the commit message 78db307bb2 says "this mechanism cannot save us".

But I'll do some testing of that. Let's ensure that the error message makes sense at the very least.

Ok, I reconstructed that scenario artificially with the attached script. It creates a new cluster, runs a test program to create a bunch of multixids, runs vacuum freeze to truncate the multixids SLRU and advance oldestMulti. Finally, it uses pg_resetwal to reset oldestMulti back to 1, to simulate what you'd get with the buggy 9.3 pg_upgrade. I tested this with a v18 source cluster, but I don't think the source version matters. (It must've been pg_upgraded to 9.3 in the past, but it could've gone through more pg_upgrades since)

When you run pg_upgrade on that, it fails like this:

Converting pg_multixact files could not open file "/tmp/pgcluster/pg_multixact/offsets/0000": No such file or directory

Not an ideal error message, but at least you get an error rather than silently losing data.

With the attached patch, you get an error message crafted specifically for that case.

Converting pg_multixact files                                 fatal

Segment file containing multixid 1 does not exist.
This can happen if an old version of pg_upgrade was used in the past
to upgrade the cluster from version 9.3 to 9.4. If that is the cause,
it can be fixed by running VACUUM FREEZE before upgrade.
Failure, exiting

I'm not sure this is worth committing though. How many clusters are there out there that have been pg_upgraded to version 9.3.0-9.3.4, and haven't since been running for long enough to fix the oldestMultiXid value? Having that error message seems pretty harmless too, but if you actually hit that case that the file is missing, it might actually be more likely that you've hit some other bug than that old pg_upgrade issue.

Another approach would be to ignore the oldestMulti in the control file and scan pg_multixact/offsets for the oldest segment that actually exists. But I don't like that. Unless you actually hit that old pg_upgrade bug, a missing file is a sign of trouble, and it's better that you get alerted about it than upgrading the broken cluster to a new version.

- Heikki

Attachment: init_multixids.sh
Description: application/shellscript

import sys
import psycopg

if len(sys.argv) != 2:
    print("Usage: python generate_multixids.py <count>")
    sys.exit(1)

count = int(sys.argv[1])

print(f"Generating {count} multixact IDs...")

# Determine per-worker iterations
num_conns = 10

conns = []
for i in range(num_conns):
    conn = psycopg.connect(host="localhost", dbname="postgres", user="postgres")
    conns.append(conn)

for i in range(count):
    if i % 1000 == 0:
        print(f"processing {i} / {count}...")

    conn = conns[i % num_conns];
    conn.commit()
    cur = conn.cursor()
    cur.execute("SELECT * FROM t WHERE id=1 FOR SHARE")

print("Done generating multixact IDs.")

From ea7af58c794321887f3492cc1b741b3614d15180 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Thu, 17 Sep 2026 18:13:54 +0300
Subject: [PATCH 1/1] Give a special error if multixid 1 is missing on
 pg_upgrade

---
 src/bin/pg_upgrade/multixact_read_v18.c | 21 ++++++-
 src/bin/pg_upgrade/multixact_read_v18.h |  2 +-
 src/bin/pg_upgrade/multixact_rewrite.c  |  6 +-
 src/bin/pg_upgrade/slru_io.c            | 74 +++++++++++++++++++------
 src/bin/pg_upgrade/slru_io.h            |  1 +
 5 files changed, 82 insertions(+), 22 deletions(-)

diff --git a/src/bin/pg_upgrade/multixact_read_v18.c b/src/bin/pg_upgrade/multixact_read_v18.c
index c92b977530a..08fce9e7186 100644
--- a/src/bin/pg_upgrade/multixact_read_v18.c
+++ b/src/bin/pg_upgrade/multixact_read_v18.c
@@ -116,7 +116,8 @@ MXOffsetToFlagsBitShift(MultiXactOffset32 offset)
  * Returns the malloced memory used by the all other calls in this module.
  */
 OldMultiXactReader *
-AllocOldMultiXactRead(char *pgdata, MultiXactId nextMulti,
+AllocOldMultiXactRead(char *pgdata,
+					  MultiXactId oldestMulti, MultiXactId nextMulti,
 					  MultiXactOffset32 nextOffset)
 {
 	OldMultiXactReader *state = pg_malloc_object(OldMultiXactReader);
@@ -131,6 +132,24 @@ AllocOldMultiXactRead(char *pgdata, MultiXactId nextMulti,
 	pg_sprintf(dir, "%s/pg_multixact/members", pgdata);
 	state->members = AllocSlruRead(dir, false);
 
+	/*
+	 * If oldestMulti is 1 (FirstMultiXactId), check that the corresponding
+	 * offsets segment exists.  You'd get an error later anyway when trying to
+	 * read it, but we want to give a special error message for that case
+	 * because there was a bug in old versions of pg_upgrade where oldestMulti
+	 * was incorrectly set to 1.  See commit a61daa14d5 that fixed that bug.
+	 */
+	if (oldestMulti == FirstMultiXactId && nextMulti != oldestMulti &&
+		!SlruReadSegmentExists(state->offset, MultiXactIdToOffsetPage(oldestMulti)))
+	{
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("Segment file containing multixid %u does not exist.\n"
+				 "This can happen if an old version of pg_upgrade was used in the\n"
+				 "past to upgrade the cluster to version 9.3. If that is the cause,\n"
+				 "it can be fixed by running VACUUM FREEZE before upgrade.",
+				 oldestMulti);
+	}
+
 	return state;
 }
 
diff --git a/src/bin/pg_upgrade/multixact_read_v18.h b/src/bin/pg_upgrade/multixact_read_v18.h
index 88b52b9d2e8..bc4b73d452c 100644
--- a/src/bin/pg_upgrade/multixact_read_v18.h
+++ b/src/bin/pg_upgrade/multixact_read_v18.h
@@ -27,7 +27,7 @@ typedef struct OldMultiXactReader
 } OldMultiXactReader;
 
 extern OldMultiXactReader *AllocOldMultiXactRead(char *pgdata,
-												 MultiXactId nextMulti,
+												 MultiXactId oldestMulti, MultiXactId nextMulti,
 												 MultiXactOffset32 nextOffset);
 extern bool GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
 										  MultiXactId multi,
diff --git a/src/bin/pg_upgrade/multixact_rewrite.c b/src/bin/pg_upgrade/multixact_rewrite.c
index c7a1416494d..c670ff36fe0 100644
--- a/src/bin/pg_upgrade/multixact_rewrite.c
+++ b/src/bin/pg_upgrade/multixact_rewrite.c
@@ -62,7 +62,7 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi)
 	 * old cluster.
 	 */
 	old_reader = AllocOldMultiXactRead(old_cluster.pgdata,
-									   old_cluster.controldata.chkpnt_nxtmulti,
+									   from_multi, old_cluster.controldata.chkpnt_nxtmulti,
 									   old_cluster.controldata.chkpnt_nxtmxoff);
 
 	for (MultiXactId multi = from_multi; multi != to_multi;)
@@ -76,8 +76,8 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi)
 		 * Locking-only XIDs that may be part of multi-xids don't matter after
 		 * upgrade, as there can be no transactions running across upgrade. So
 		 * as a small optimization, we only read one member from each
-		 * multixid: the one updating one, or if there was no update,
-		 * arbitrarily the first locking xid.
+		 * multixid: the updating one, or if there was no update, arbitrarily
+		 * the first locking xid.
 		 */
 		multixid_valid = GetOldMultiXactIdSingleMember(old_reader, multi, &member);
 
diff --git a/src/bin/pg_upgrade/slru_io.c b/src/bin/pg_upgrade/slru_io.c
index aa9d59a0d7b..caa1a1859b2 100644
--- a/src/bin/pg_upgrade/slru_io.c
+++ b/src/bin/pg_upgrade/slru_io.c
@@ -20,6 +20,8 @@
 
 static SlruSegState *AllocSlruSegState(const char *dir);
 static char *SlruFileName(SlruSegState *state, int64 segno);
+static bool SlruReadSwitchSegment(SlruSegState *state, int64 segno, bool missing_ok);
+static void SlruReadCloseSegment(SlruSegState *state);
 static void SlruFlush(SlruSegState *state);
 
 /* common parts of AllocSlruRead and AllocSlruWrite */
@@ -69,6 +71,49 @@ AllocSlruRead(const char *dir, bool long_segment_names)
 	return state;
 }
 
+/*
+ * Open the given segment, closing old one first if required.
+ *
+ * If 'missing_ok' is true and the file does not exist, returns false.
+ * Otherwise a missing file is fatal.
+ */
+static bool
+SlruReadSwitchSegment(SlruSegState *state, int64 segno, bool missing_ok)
+{
+	SlruReadCloseSegment(state);
+
+	state->fn = SlruFileName(state, segno);
+	if ((state->fd = open(state->fn, O_RDONLY | PG_BINARY, 0)) < 0)
+	{
+		if (missing_ok && errno == ENOENT)
+		{
+			pg_free(state->fn);
+			return false;
+		}
+		pg_fatal("could not open file \"%s\": %m", state->fn);
+	}
+	state->segno = segno;
+	return true;
+}
+
+/*
+ * Close the current segment file, if any.
+ */
+static void
+SlruReadCloseSegment(SlruSegState *state)
+{
+	if (state->segno != -1)
+	{
+		close(state->fd);
+		state->fd = -1;
+
+		pg_free(state->fn);
+		state->fn = NULL;
+
+		state->segno = -1;
+	}
+}
+
 /*
  * Read the given page into memory buffer.
  *
@@ -96,23 +141,7 @@ SlruReadSwitchPageSlow(SlruSegState *state, uint64 pageno)
 	/* If the new page is on a different SLRU segment, open the new segment */
 	segno = pageno / SLRU_PAGES_PER_SEGMENT;
 	if (segno != state->segno)
-	{
-		if (state->segno != -1)
-		{
-			close(state->fd);
-			state->fd = -1;
-
-			pg_free(state->fn);
-			state->fn = NULL;
-
-			state->segno = -1;
-		}
-
-		state->fn = SlruFileName(state, segno);
-		if ((state->fd = open(state->fn, O_RDONLY | PG_BINARY, 0)) < 0)
-			pg_fatal("could not open file \"%s\": %m", state->fn);
-		state->segno = segno;
-	}
+		SlruReadSwitchSegment(state, segno, false);
 
 	offset = (pageno % SLRU_PAGES_PER_SEGMENT) * BLCKSZ;
 	bytes_read = 0;
@@ -146,6 +175,17 @@ SlruReadSwitchPageSlow(SlruSegState *state, uint64 pageno)
 	return state->buf.data;
 }
 
+/*
+ * Return true if the segment file containing given page exists.
+ */
+bool
+SlruReadSegmentExists(SlruSegState *state, uint64 pageno)
+{
+	int64		segno = pageno / SLRU_PAGES_PER_SEGMENT;
+
+	return SlruReadSwitchSegment(state, segno, true);
+}
+
 /*
  * Free the reader.
  */
diff --git a/src/bin/pg_upgrade/slru_io.h b/src/bin/pg_upgrade/slru_io.h
index 0be83b28615..de871e37641 100644
--- a/src/bin/pg_upgrade/slru_io.h
+++ b/src/bin/pg_upgrade/slru_io.h
@@ -26,6 +26,7 @@ typedef struct SlruSegState
 } SlruSegState;
 
 extern SlruSegState *AllocSlruRead(const char *dir, bool long_segment_names);
+extern bool SlruReadSegmentExists(SlruSegState *state, uint64 pageno);
 extern char *SlruReadSwitchPageSlow(SlruSegState *state, uint64 pageno);
 extern void FreeSlruRead(SlruSegState *state);
 
-- 
2.47.3

Reply via email to