public inbox for [email protected]
help / color / mirror / Atom feedFrom: Maxim Orlov <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: wenhui qiu <[email protected]>
Cc: Postgres hackers <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Subject: Re: POC: make mxidoff 64 bits
Date: Thu, 4 Dec 2025 18:33:47 +0300
Message-ID: <CACG=eza-JrdrWqCu2zR05+jWPSHO2CO9hrHTYBNF9LSd4dzXGw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CACG=ezaWg7_nt-8ey4aKv2w9LcuLthHknwCawmBgEeTnJrJTcw@mail.gmail.com>
<CACG=ezZwdvsijzuXE3hex3xHcoz75EQYBXRTsQJVwbo5J5sS3g@mail.gmail.com>
<CACG=ezbs912S58=uR17b4w8uuWv1=OcCRaTW_OWdFm4+tXZA6w@mail.gmail.com>
<CAGjGUA+BfcWyccNN4=tHsW_E-koRxbg8h8ut6hjvPsHMgmek6w@mail.gmail.com>
<CACG=ezYbYO_KHWdeDedbDcY0tOS0JfaqBxG3=bG5+DdsDK4MpQ@mail.gmail.com>
<CACG=ezYpZRPwoRCz_h3Qerd3XJNdpTHCpwGbZphNdy26tA4_qQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<CACG=ezYUJSvnuxntkURNWo_1vZ+AtmcQfqd_h6WgDzGaudfw+Q@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAExHW5tUEkiQrvm9hgccjKUNkWBnJ5_HDUrAwiHBTxu+Vuj29Q@mail.gmail.com>
<[email protected]>
<[email protected]>
<CACG=ezY0=ri8A0duXbpd1XNUc1jnngaPnmB0-+UZpxAv7-fNtw@mail.gmail.com>
<[email protected]>
<[email protected]>
On Thu, 4 Dec 2025 at 13:39, Heikki Linnakangas <[email protected]> wrote:
> However, that doesn't hold for pg_upgrade. pg_upgrade will try to read
> all the multixids. So we need to make the multixact reading code
> tolerant of the situations that could be present after a crash. I think
> the right philosophy here is that we try to read all the old multixids,
> and do our best to interpret them the same way that the old server
> would.
Something like attached?
Now previous scheme of upgrade with the bytes joggling start
looking not so bad. Just a funny thought that came to my mind.
Perhaps we should check that all the files exist and have the correct
sizes in the pre-check stage
Not sure about it. Because SLRU does not support "holes", simply
checking if the first and last multixacts exist will be enough. But
we'll do it anyway in a real conversion.
PFA to start a conversation.
--
Best regards,
Maxim Orlov.
From c2ccb107bef898420e6417c37d56c6b30578d28f Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Thu, 4 Dec 2025 17:02:35 +0300
Subject: [PATCH] rough draft of skipping bogus offsets
---
src/bin/pg_upgrade/multixact_old.c | 38 ++++++++++++++++++++++++------
src/bin/pg_upgrade/multixact_old.h | 2 +-
src/bin/pg_upgrade/pg_upgrade.c | 15 ++++++------
3 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/src/bin/pg_upgrade/multixact_old.c b/src/bin/pg_upgrade/multixact_old.c
index 529eeeb93b6..685bfaeff82 100644
--- a/src/bin/pg_upgrade/multixact_old.c
+++ b/src/bin/pg_upgrade/multixact_old.c
@@ -136,7 +136,7 @@ AllocOldMultiXactRead(char *pgdata, MultiXactId nextMulti,
* - Because there's no concurrent activity, We don't need to worry about
* locking and some corner cases.
*/
-void
+bool
GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
TransactionId *result, MultiXactStatus *status)
{
@@ -189,7 +189,18 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
offptr += entryno;
offset = *offptr;
- Assert(offset != 0);
+ if (offset == 0)
+ {
+ pg_log(PG_WARNING, "multixact %u, offset is empty", multi);
+ return false;
+ }
+#if 0
+ if ( <more checks> )
+ {
+ pg_log(PG_WARNING, "multixact %u, offset is bogus", multi);
+ return false;
+ }
+#endif
/*
* Use the same increment rule as GetNewMultiXactId(), that is, don't
@@ -224,9 +235,13 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
/*
* Corner case 2: next multixact is still being filled in, this cannot
- * happen during upgrade.
+ * happen during upgrade, but if it does, complain.
*/
- Assert(nextMXOffset != 0);
+ if (nextMXOffset == 0)
+ {
+ pg_log(PG_WARNING, "multixact next to %u is empty", multi);
+ return false;
+ }
length = nextMXOffset - offset;
}
@@ -272,8 +287,11 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
{
/* sanity check */
if (result_isupdate)
- pg_fatal("multixact %u has more than one updating member",
- multi);
+ {
+ pg_log(PG_WARNING,
+ "multixact %u has more than one updating member", multi);
+ return false;
+ }
result_xid = *xactptr;
result_isupdate = true;
}
@@ -282,11 +300,17 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
}
/* A multixid with zero members should not happen */
- Assert(TransactionIdIsValid(result_xid));
+ if (!TransactionIdIsValid(result_xid))
+ {
+ pg_log(PG_WARNING, "multixact %u have zero members", multi);
+ return false;
+ }
*result = result_xid;
*status = result_isupdate ? MultiXactStatusUpdate :
MultiXactStatusForKeyShare;
+
+ return true;
}
/*
diff --git a/src/bin/pg_upgrade/multixact_old.h b/src/bin/pg_upgrade/multixact_old.h
index 4f9e086a1fb..b7352159d83 100644
--- a/src/bin/pg_upgrade/multixact_old.h
+++ b/src/bin/pg_upgrade/multixact_old.h
@@ -29,7 +29,7 @@ typedef struct OldMultiXactReader
extern OldMultiXactReader *AllocOldMultiXactRead(char *pgdata,
MultiXactId nextMulti,
OldMultiXactOffset nextOffset);
-extern void GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
+extern bool GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
MultiXactId multi,
TransactionId *result,
MultiXactStatus *status);
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index ff937b9e104..c5da56fe785 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -832,14 +832,15 @@ convert_multixacts(MultiXactId from_multi, MultiXactId to_multi)
* one updating one, or if there was no update, arbitrarily the
* first locking xid.
*/
- GetOldMultiXactIdSingleMember(old_reader, multi, &xid, &status);
+ if (GetOldMultiXactIdSingleMember(old_reader, multi, &xid, &status))
+ {
+ /* Write it out in new format */
+ member.xid = xid;
+ member.status = status;
+ RecordNewMultiXact(new_writer, next_offset, multi, 1, &member);
+ next_offset += 1;
+ }
- /* Write it out in new format */
- member.xid = xid;
- member.status = status;
- RecordNewMultiXact(new_writer, next_offset, multi, 1, &member);
-
- next_offset += 1;
multi++;
/* handle wraparound */
if (multi < FirstMultiXactId)
--
2.43.0
From 0ac8eb292c21a06da31215aa41adb53ec1f90872 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Thu, 4 Dec 2025 18:31:37 +0300
Subject: [PATCH 2/2] Check is first and last multis exists
---
src/bin/pg_upgrade/multixact_old.c | 10 ++++++++++
src/bin/pg_upgrade/multixact_old.h | 2 ++
src/bin/pg_upgrade/pg_upgrade.c | 23 +++++++++++++++++++++++
3 files changed, 35 insertions(+)
diff --git a/src/bin/pg_upgrade/multixact_old.c b/src/bin/pg_upgrade/multixact_old.c
index 685bfaeff82..ffd06ad908f 100644
--- a/src/bin/pg_upgrade/multixact_old.c
+++ b/src/bin/pg_upgrade/multixact_old.c
@@ -324,3 +324,13 @@ FreeOldMultiXactReader(OldMultiXactReader *state)
pfree(state);
}
+
+void
+CheckOldMultiXactIdExist(OldMultiXactReader *state, MultiXactId multi)
+{
+ int64 pageno = MultiXactIdToOffsetPage(multi);
+ char *buf = SlruReadSwitchPage(state->offset, pageno);
+
+ if (!buf)
+ pg_fatal("could not read multixact %u", multi);
+}
diff --git a/src/bin/pg_upgrade/multixact_old.h b/src/bin/pg_upgrade/multixact_old.h
index b7352159d83..86141ac392f 100644
--- a/src/bin/pg_upgrade/multixact_old.h
+++ b/src/bin/pg_upgrade/multixact_old.h
@@ -34,5 +34,7 @@ extern bool GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
TransactionId *result,
MultiXactStatus *status);
extern void FreeOldMultiXactReader(OldMultiXactReader *reader);
+extern void CheckOldMultiXactIdExist(OldMultiXactReader *state,
+ MultiXactId multi);
#endif /* MULTIXACT_OLD_H */
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index c5da56fe785..647e05f350a 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -772,6 +772,21 @@ copy_subdir_files(const char *old_subdir, const char *new_subdir)
check_ok();
}
+static void
+check_multixacts(MultiXactId from_multi, MultiXactId to_multi)
+{
+ OldMultiXactReader *reader;
+
+ reader = AllocOldMultiXactRead(old_cluster.pgdata,
+ old_cluster.controldata.chkpnt_nxtmulti,
+ old_cluster.controldata.chkpnt_nxtmxoff);
+
+ CheckOldMultiXactIdExist(reader, from_multi);
+ CheckOldMultiXactIdExist(reader, to_multi);
+
+ FreeOldMultiXactReader(reader);
+}
+
/*
* Convert pg_multixact/offset and /members from the old format with 32-bit
* offsets.
@@ -958,6 +973,14 @@ copy_xact_xlog_xid(void)
remove_new_subdir("pg_multixact/members", false);
remove_new_subdir("pg_multixact/offsets", false);
+ /*
+ * Before the actual conversion do sanity check.
+ * XXX: place it properly, it should be better place for this
+ */
+ prep_status("Sanity check pg_multixact files");
+ check_multixacts(oldstMulti, nxtmulti);
+ check_ok();
+
/*
* Create new pg_multixact files, converting old ones if needed.
*/
--
2.43.0
Attachments:
[text/plain] 0001-rough-draft-of-skipping-bogus-offsets.patch.txt (4.3K, 3-0001-rough-draft-of-skipping-bogus-offsets.patch.txt)
download | inline diff:
From c2ccb107bef898420e6417c37d56c6b30578d28f Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Thu, 4 Dec 2025 17:02:35 +0300
Subject: [PATCH] rough draft of skipping bogus offsets
---
src/bin/pg_upgrade/multixact_old.c | 38 ++++++++++++++++++++++++------
src/bin/pg_upgrade/multixact_old.h | 2 +-
src/bin/pg_upgrade/pg_upgrade.c | 15 ++++++------
3 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/src/bin/pg_upgrade/multixact_old.c b/src/bin/pg_upgrade/multixact_old.c
index 529eeeb93b6..685bfaeff82 100644
--- a/src/bin/pg_upgrade/multixact_old.c
+++ b/src/bin/pg_upgrade/multixact_old.c
@@ -136,7 +136,7 @@ AllocOldMultiXactRead(char *pgdata, MultiXactId nextMulti,
* - Because there's no concurrent activity, We don't need to worry about
* locking and some corner cases.
*/
-void
+bool
GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
TransactionId *result, MultiXactStatus *status)
{
@@ -189,7 +189,18 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
offptr += entryno;
offset = *offptr;
- Assert(offset != 0);
+ if (offset == 0)
+ {
+ pg_log(PG_WARNING, "multixact %u, offset is empty", multi);
+ return false;
+ }
+#if 0
+ if ( <more checks> )
+ {
+ pg_log(PG_WARNING, "multixact %u, offset is bogus", multi);
+ return false;
+ }
+#endif
/*
* Use the same increment rule as GetNewMultiXactId(), that is, don't
@@ -224,9 +235,13 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
/*
* Corner case 2: next multixact is still being filled in, this cannot
- * happen during upgrade.
+ * happen during upgrade, but if it does, complain.
*/
- Assert(nextMXOffset != 0);
+ if (nextMXOffset == 0)
+ {
+ pg_log(PG_WARNING, "multixact next to %u is empty", multi);
+ return false;
+ }
length = nextMXOffset - offset;
}
@@ -272,8 +287,11 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
{
/* sanity check */
if (result_isupdate)
- pg_fatal("multixact %u has more than one updating member",
- multi);
+ {
+ pg_log(PG_WARNING,
+ "multixact %u has more than one updating member", multi);
+ return false;
+ }
result_xid = *xactptr;
result_isupdate = true;
}
@@ -282,11 +300,17 @@ GetOldMultiXactIdSingleMember(OldMultiXactReader *state, MultiXactId multi,
}
/* A multixid with zero members should not happen */
- Assert(TransactionIdIsValid(result_xid));
+ if (!TransactionIdIsValid(result_xid))
+ {
+ pg_log(PG_WARNING, "multixact %u have zero members", multi);
+ return false;
+ }
*result = result_xid;
*status = result_isupdate ? MultiXactStatusUpdate :
MultiXactStatusForKeyShare;
+
+ return true;
}
/*
diff --git a/src/bin/pg_upgrade/multixact_old.h b/src/bin/pg_upgrade/multixact_old.h
index 4f9e086a1fb..b7352159d83 100644
--- a/src/bin/pg_upgrade/multixact_old.h
+++ b/src/bin/pg_upgrade/multixact_old.h
@@ -29,7 +29,7 @@ typedef struct OldMultiXactReader
extern OldMultiXactReader *AllocOldMultiXactRead(char *pgdata,
MultiXactId nextMulti,
OldMultiXactOffset nextOffset);
-extern void GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
+extern bool GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
MultiXactId multi,
TransactionId *result,
MultiXactStatus *status);
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index ff937b9e104..c5da56fe785 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -832,14 +832,15 @@ convert_multixacts(MultiXactId from_multi, MultiXactId to_multi)
* one updating one, or if there was no update, arbitrarily the
* first locking xid.
*/
- GetOldMultiXactIdSingleMember(old_reader, multi, &xid, &status);
+ if (GetOldMultiXactIdSingleMember(old_reader, multi, &xid, &status))
+ {
+ /* Write it out in new format */
+ member.xid = xid;
+ member.status = status;
+ RecordNewMultiXact(new_writer, next_offset, multi, 1, &member);
+ next_offset += 1;
+ }
- /* Write it out in new format */
- member.xid = xid;
- member.status = status;
- RecordNewMultiXact(new_writer, next_offset, multi, 1, &member);
-
- next_offset += 1;
multi++;
/* handle wraparound */
if (multi < FirstMultiXactId)
--
2.43.0
[text/plain] 0002-Check-is-first-and-last-multis-exists.patch.txt (2.8K, 4-0002-Check-is-first-and-last-multis-exists.patch.txt)
download | inline diff:
From 0ac8eb292c21a06da31215aa41adb53ec1f90872 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Thu, 4 Dec 2025 18:31:37 +0300
Subject: [PATCH 2/2] Check is first and last multis exists
---
src/bin/pg_upgrade/multixact_old.c | 10 ++++++++++
src/bin/pg_upgrade/multixact_old.h | 2 ++
src/bin/pg_upgrade/pg_upgrade.c | 23 +++++++++++++++++++++++
3 files changed, 35 insertions(+)
diff --git a/src/bin/pg_upgrade/multixact_old.c b/src/bin/pg_upgrade/multixact_old.c
index 685bfaeff82..ffd06ad908f 100644
--- a/src/bin/pg_upgrade/multixact_old.c
+++ b/src/bin/pg_upgrade/multixact_old.c
@@ -324,3 +324,13 @@ FreeOldMultiXactReader(OldMultiXactReader *state)
pfree(state);
}
+
+void
+CheckOldMultiXactIdExist(OldMultiXactReader *state, MultiXactId multi)
+{
+ int64 pageno = MultiXactIdToOffsetPage(multi);
+ char *buf = SlruReadSwitchPage(state->offset, pageno);
+
+ if (!buf)
+ pg_fatal("could not read multixact %u", multi);
+}
diff --git a/src/bin/pg_upgrade/multixact_old.h b/src/bin/pg_upgrade/multixact_old.h
index b7352159d83..86141ac392f 100644
--- a/src/bin/pg_upgrade/multixact_old.h
+++ b/src/bin/pg_upgrade/multixact_old.h
@@ -34,5 +34,7 @@ extern bool GetOldMultiXactIdSingleMember(OldMultiXactReader *state,
TransactionId *result,
MultiXactStatus *status);
extern void FreeOldMultiXactReader(OldMultiXactReader *reader);
+extern void CheckOldMultiXactIdExist(OldMultiXactReader *state,
+ MultiXactId multi);
#endif /* MULTIXACT_OLD_H */
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index c5da56fe785..647e05f350a 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -772,6 +772,21 @@ copy_subdir_files(const char *old_subdir, const char *new_subdir)
check_ok();
}
+static void
+check_multixacts(MultiXactId from_multi, MultiXactId to_multi)
+{
+ OldMultiXactReader *reader;
+
+ reader = AllocOldMultiXactRead(old_cluster.pgdata,
+ old_cluster.controldata.chkpnt_nxtmulti,
+ old_cluster.controldata.chkpnt_nxtmxoff);
+
+ CheckOldMultiXactIdExist(reader, from_multi);
+ CheckOldMultiXactIdExist(reader, to_multi);
+
+ FreeOldMultiXactReader(reader);
+}
+
/*
* Convert pg_multixact/offset and /members from the old format with 32-bit
* offsets.
@@ -958,6 +973,14 @@ copy_xact_xlog_xid(void)
remove_new_subdir("pg_multixact/members", false);
remove_new_subdir("pg_multixact/offsets", false);
+ /*
+ * Before the actual conversion do sanity check.
+ * XXX: place it properly, it should be better place for this
+ */
+ prep_status("Sanity check pg_multixact files");
+ check_multixacts(oldstMulti, nxtmulti);
+ check_ok();
+
/*
* Create new pg_multixact files, converting old ones if needed.
*/
--
2.43.0
view thread (79+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: POC: make mxidoff 64 bits
In-Reply-To: <CACG=eza-JrdrWqCu2zR05+jWPSHO2CO9hrHTYBNF9LSd4dzXGw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox