| From: | Sami Imseih <samimseih(at)gmail(dot)com> |
|---|---|
| To: | Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> |
| Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Re: Fix bug in multixact Oldest*MXactId initialization and access |
| Date: | 2026-02-25 01:02:34 |
| Message-ID: | CAA5RZ0tnbfDNo6FCNhn0mmxRgFzS5tALPpJ28aT7oevwp6NFOw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
> But proc structs reserved for two phase commit are placed after auxiliary
> procs, therefore writes to OldestMemberMXactId[dummy] starts to overwrites
> slots of OldestVisibleMXactId.
>
> Then PostgreSQL 18 increased NUM_AUXILIARY_PROCS due to reserve of workers
> for AIO. And it is possible to make such test postgresql.conf with so
> extremely low MaxBackend so writes to OldestMemberMXactId[dummy] overwrites
> first entry of BufferDescriptors, which are allocated next in shared memory.
From what I can tell, PreparedXactProcs proc numbers start at
MaxBackends + NUM_AUXILIARY_PROCS, but MaxOldestSlot is calculated as
MaxBackends + max_prepared_xacts, missing the NUM_AUXILIARY_PROCS offset.
MyProcNumber (for regular backends) is always < MaxBackends, so it's safe.
But dummyProcNumber (for prepared transactions) starts at
MaxBackends + NUM_AUXILIARY_PROCS, which exceeds MaxOldestSlot.
Using this workload:
```
psql -c "DROP TABLE IF EXISTS test;"
psql -c "CREATE TABLE IF NOT EXISTS test (id int PRIMARY KEY, data text);"
psql -c "INSERT INTO test VALUES (1, 'test data');"
for i in {1..25}; do
psql -q <<EOF
BEGIN;
SELECT * FROM test WHERE id = 1 FOR SHARE;
PREPARE TRANSACTION 'prep_$i';
EOF
echo " Created prep_$i"
done
```
I can see the assert in your patch fail here:
```
@@ -1628,8 +1631,8 @@ PostPrepare_MultiXact(FullTransactionId fxid)
*/
LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE);
- OldestMemberMXactId[dummyProcNumber] = myOldestMember;
- OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId;
+ setOldest(OldestMemberMXactId, dummyProcNumber, myOldestMember);
```
> Patch in attach replaces direct accesses to this arrays with inline
> functions which include asserts. And changes calculation of MaxOldestSlot
> to include NUM_AUXILIARY_PROCS.
This is the important part of the patch to correctly size MaxOldestSlot
```
+#define MaxOldestSlot (MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts)
```
but, I am not sure if we need all the extra asserts, as this can only
be an issue for the
dummyProcNumber, right?
Maybe we can also define a new macro:
```
#define TOTAL_PROCS MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts
```
and use it in InitProcGlobal instead of TotalProcs, as well as MaxOldestSlot?
--
Sami Imseih
Amazon Web Services
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-02-25 01:30:29 | Re: pgsql: libpq: Grease the protocol by default |
| Previous Message | David Rowley | 2026-02-25 00:59:53 | Re: More speedups for tuple deformation |