On 03/12/2025 19:19, Heikki Linnakangas wrote:
On 03/12/2025 16:19, Dmitry Yurichev wrote:
On 12/2/25 16:48, Heikki Linnakangas wrote:
Thanks! Agreed, v14-16 were the same. v17 and v18 might be worth
testing separately, to make sure I didn't e.g. screw up the locking
differences.
I tested on the REL_18_1 tag (with applying v15-pg18-0001-Set-next-
multixid-s-offset-when-creating-a-.patch).
I didn't notice any deadlocks or other errors. Thanks!
Okay. I fixed a few more little things:
- fixed the warnings about shadowed variables that Andrey pointed out
- in older branches before we switched to 64-bit SLRU page numbers, use
'int' rather than 'int64' in page number variables
- improve wording and fix few trivial typos in comments
Committed with those last-minute changes. Thanks for the testing!
While working on the 64-bit multixid offsets patch, I noticed one more
bug with this. At offset wraparound, when we set the next multixid's
offset in RecordNewMultiXact, we incorrectly set it to 0 instead of 1.
We're supposed to skip over offset 1, because 0 is reserved to mean
invalid. We do that correctly when setting the "current" multixid's
offset, because the caller of RecordNewMultiXact has already skipped
over offset 0, but I missed it for the next offset.
I was able to reproduce that with these steps:
pg_resetwal -O 0xffffff00 -D data
pg_ctl -D data start
pgbench -s5 -i postgres
pgbench -c3 -t100 -f a.sql postgres
a.sql:
select * from pgbench_branches FOR KEY SHARE;
You get an error like this:
pgbench: error: client 2 script 0 aborted in command 0 query 0: ERROR:
MultiXact 372013 has invalid next offset
I tried to modify the new wraparound TAP test to reproduce that, but it
turned out to be difficult because you need to have multiple backends
assigning multixids concurrently to hit that.
The fix is pretty straightforward, see attached. Barring objections,
I'll commit and backport this.
- Heikki
From f9b2cc8daea1cd0462e9455736237da0f7f00a3d Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Thu, 4 Dec 2025 16:05:13 +0200
Subject: [PATCH 1/1] Fix setting next multixid's offset at offset wraparound
---
src/backend/access/transam/multixact.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 27f02faec80..8ed3fd9d071 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -909,6 +909,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
int64 next_pageno;
int next_entryno;
MultiXactOffset *next_offptr;
+ MultiXactOffset next_offset;
LWLock *lock;
LWLock *prevlock = NULL;
@@ -976,11 +977,15 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
next_offptr += next_entryno;
}
- if (*next_offptr != offset + nmembers)
+ /* Like in GetNewMultiXactId(), skip over offset 0 */
+ next_offset = offset + nmembers;
+ if (next_offset == 0)
+ next_offset = 1;
+ if (*next_offptr != next_offset)
{
/* should already be set to the correct value, or not at all */
Assert(*next_offptr == 0);
- *next_offptr = offset + nmembers;
+ *next_offptr = next_offset;
MultiXactOffsetCtl->shared->page_dirty[slotno] = true;
}
--
2.47.3
