Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add test for row-locking and multixids with prepared transactions

  2. Skip prepared_xacts test if max_prepared_transactions < 2

  3. Fix OldestMemberMXactId and OldestVisibleMXactId array usage

  4. Redefine backend ID to be an index into the proc array

  5. Fix an old bug in multixact and two-phase commit. Prepared transactions can

  1. Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-24T17:35:18Z

    Good day.

multixact.c has bug in initialization and access of OldestMemberMXactId
(and partially OldestVisibleMXactId).

Size of this arrays is defined as:

 #define MaxOldestSlot (MaxBackends + max_prepared_xacts)

assuming there are only backends and prepared transactions could hold
multixacts.

This assumption is correct. And in fact there were no bug when this formula
were introduced in 2009y [1], since these arrays were indexed but synthetic
dummy `dummyBackendId` field of `GlobalTransactionData` struct.

But in 2024y [2] field `dummyBackendId` were removed and pgprocno were used
instead.

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.

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.

Certainly, it is not clearest patch possible:
- may be you will decide to not introduce inline functions,
- or will introduce separate inline function for each array,
- or will fix slot calculation to remove aux procs from account,
- or will revert deletion of dummyBackendId.

[1]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=cd87b6f8a5084c070c3e56b07794be8fea33647d
[2]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ab355e3a88de745607f6dd4c21f0119b5c68f2ad

-- 
regards
Yura Sokolov aka funny-falcon

  2. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-02-25T01:02:34Z

    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

  3. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Chao Li <li.evan.chao@gmail.com> — 2026-02-25T01:44:22Z

    
> On Feb 25, 2026, at 01:35, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
> 
> Good day.
> 
> multixact.c has bug in initialization and access of OldestMemberMXactId
> (and partially OldestVisibleMXactId).
> 
> Size of this arrays is defined as:
> 
> #define MaxOldestSlot (MaxBackends + max_prepared_xacts)
> 
> assuming there are only backends and prepared transactions could hold
> multixacts.
> 
> This assumption is correct. And in fact there were no bug when this formula
> were introduced in 2009y [1], since these arrays were indexed but synthetic
> dummy `dummyBackendId` field of `GlobalTransactionData` struct.
> 
> But in 2024y [2] field `dummyBackendId` were removed and pgprocno were used
> instead.
> 
> 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.
> 
> 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.
> 
> Certainly, it is not clearest patch possible:
> - may be you will decide to not introduce inline functions,
> - or will introduce separate inline function for each array,
> - or will fix slot calculation to remove aux procs from account,
> - or will revert deletion of dummyBackendId.
> 
> [1]
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=cd87b6f8a5084c070c3e56b07794be8fea33647d
> [2]
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ab355e3a88de745607f6dd4c21f0119b5c68f2ad
> 
> -- 
> regards
> Yura Sokolov aka funny-falcon
> <v00-0001-Fix-multixacts-OldestMemberMXactId-and-OldestVis.patch>


I think this is a good catch. I read through the related code paths:

In InitProcGlobal()
```
    PreparedXactProcs = &procs[MaxBackends + NUM_AUXILIARY_PROCS];
```
Then in TwoPhaseShmemInit()
```
    gxacts[i].pgprocno = GetNumberFromPGProc(&PreparedXactProcs[i]);
```
This shows prepared xacts are placed after MaxBackends + NUM_AUXILIARY_PROCS.

Also, in InitializeMaxBackends():
```
    /* Note that this does not include "auxiliary" processes */
    MaxBackends = MaxConnections + autovacuum_worker_slots +
        max_worker_processes + max_wal_senders + NUM_SPECIAL_WORKER_PROCS;
```
So MaxBackends does not include NUM_AUXILIARY_PROCS.

Overall, the change looks correct to me.

One minor thought on the new helper asserts:
```
+	Assert(procno < MaxOldestSlot);
```
It may be worth also checking procno >= 0, since INVALID_PROC_NUMBER is -1.

While reading, I also noticed two stale comments related to this area. I added those fixes in 0002 (attached v2); 0001 is unchanged from the original v00.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  4. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-25T15:16:27Z

    Good day.

Chao Li and Sami Imseih, thank you for looking at.

After thinking a bit, I've decided to make sizes of arrays precise:
- OldestMemberMXactId's size remains MaxBackends + max_prepared_xacts.
  Instead of changing its size, procno is now adjusted to not include
auxiliary procs.
- OldestVisibleMXactId contains only MaxBackends elemenents now.
  It is used only for real backends and not prepared transactions.

All accesses are validated with asserts certainly.

I believe, index transformation in access of OldestMemberMXactId will not
cost much since all this operations are quite rare.
In the loops arrays are accessed directly since limiting loop index is enough.

-- 
regards
Yura Sokolov aka funny-falcon

  5. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-02-25T21:53:09Z

    Hi.

> All accesses are validated with asserts certainly.

Maybe I am missing something, but the extra asserts and wrapper
functions being proposed seem unnecessary for this purpose.
We know the total number of procs, so we just need to make this
available to all code paths that need to index by pgprocno.

If that calculation ever changes, code like multixact.c will not
need to care.

This can be done, as mentioned earlier, by defining the total
number of procs in proc.h, which can then be used by both
multixact.c and InitProcGlobal.

```
#define TOTAL_PROCS (MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts)
```

and to calculate where the prepared transaction procs start:

```
PreparedXactProcs = &procs[TOTAL_PROCS - max_prepared_xacts];
```

What do you think?

With regards to v3, I got a compilation warning that `GetOldestVisibleMXactId`
is an unused function.

--
Sami Imseih
Amazon Web Services (AWS)

  6. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Chao Li <li.evan.chao@gmail.com> — 2026-02-26T02:21:10Z

    
> On Feb 25, 2026, at 23:16, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
> 
> Good day.
> 
> Chao Li and Sami Imseih, thank you for looking at.
> 
> After thinking a bit, I've decided to make sizes of arrays precise:
> - OldestMemberMXactId's size remains MaxBackends + max_prepared_xacts.
>  Instead of changing its size, procno is now adjusted to not include
> auxiliary procs.
> - OldestVisibleMXactId contains only MaxBackends elemenents now.
>  It is used only for real backends and not prepared transactions.
> 
> All accesses are validated with asserts certainly.
> 
> I believe, index transformation in access of OldestMemberMXactId will not
> cost much since all this operations are quite rare.
> In the loops arrays are accessed directly since limiting loop index is enough.
> 
> -- 
> regards
> Yura Sokolov aka funny-falcon<v3-0001-Fix-multixacts-OldestMemberMXactId-and-OldestVisi.patch>

Actually, while I reviewing v1, I was thinking over the same way as v3, as the NUM_AUXILIARY_PROCS portions in OldestMemberMXactId and OldestVisibleMXactId arrays are not used at all. I didn’t raise the idea because that given these code are in hot paths, I wasn't sure if the complexity was worth the shared memory optimization. It’s a classic trade-off, and I’m curious to see which direction the senior committers prefer.

However, I agree that the new helper function names in v3 are a significant improvement over v1.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  7. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-26T16:22:06Z

    Good day.

Sami Imseih, I've introduced new precalculated variables:

    MaxChildren = MaxBackends + NUM_AUXILIARY_PROCS;
    TotalProcs = MaxChildren + max_prepared_xacts;
    TotalXactProcs = MaxBackends + max_prepared_xacts;

and spread their usage through the sources.
I don't know how committers will accept this change. But they will
certainly make their own version of patch, so I don't bother much.

But I still use inline procs for access to the arrays. Asserts cost nothing
in release build. And new version of functions doesn't branch.

And you're right: looks like there were mistake in previous version so
GetOldestVisibleMXactId became unused. New version has no this mistake.

Chao Li, new access functions doen't branch because I've made separate
functions to store at slots for prepared transactions.

--

regards
Yura Sokolov aka funny-falcon

  8. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-02-26T16:34:17Z

    > But I still use inline procs for access to the arrays. Asserts cost nothing
> in release build. And new version of functions doesn't branch.

Asserts may have minimal impact for most builds, but my concern is
this is all unnecessary complexity. I mean if we guarantee that the
arrays are initialized correctly, maybe we can have an assert at
init time, why would we need asserts at run time?

--
Sami Imseih
Amazon Web Services (AWS)

  9. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-26T17:02:38Z

    26.02.2026 19:34, Sami Imseih пишет:
>> But I still use inline procs for access to the arrays. Asserts cost nothing
>> in release build. And new version of functions doesn't branch.
> 
> Asserts may have minimal impact for most builds, but my concern is
> this is all unnecessary complexity. I mean if we guarantee that the
> arrays are initialized correctly, maybe we can have an assert at
> init time, why would we need asserts at run time?

They are future proof.

If there had been assertions from the beginning, there would not have been
a breaking change. The tests would have failed.

(Sorry, I've used Google Translate to write this sentence).

When you write assert, you protect yourself from shooting your leg far in
the future. Believe me.

-- 
regards
Yura Sokolov aka funny-falcon

  10. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-02-26T20:44:32Z

    > (Sorry, I've used Google Translate to write this sentence).
>
> When you write assert, you protect yourself from shooting your leg far in
> the future. Believe me.

The issue to me seems that we a few code paths that rely on the
total proc calculation in several places, and changing one and
forgetting to change another is what broke things. I am all for
future proofing this, and I think simply centralizing the calculation
should be enough without adding complexity to the code.

Maybe others have a different opinion.

--
Sami Imseih
Amazon Web Services (AWS)

  11. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-27T05:00:28Z

    26.02.2026 23:44, Sami Imseih пишет:
>> (Sorry, I've used Google Translate to write this sentence).
>>
>> When you write assert, you protect yourself from shooting your leg far in
>> the future. Believe me.
> 
> The issue to me seems that we a few code paths that rely on the
> total proc calculation in several places, and changing one and
> forgetting to change another is what broke things.

If there were asserts, then tests would have failed.
Period.

There would no the bug. There would no this discussion.
If only array bounds were checked with asserts.

-- 
regards
Yura Sokolov aka funny-falcon

  12. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Chao Li <li.evan.chao@gmail.com> — 2026-02-27T07:48:17Z

    
> On Feb 27, 2026, at 00:22, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
> 
> 
> Chao Li, new access functions doen't branch because I've made separate
> functions to store at slots for prepared transactions.
> 

```
+	OldestMemberMXactId[procno - MaxChildren + MaxBackends] = mxact;
```

Wow, this is clever. It removes NUM_AUXILIARY_PROCS from procno without introducing an extra if check.

However, I am not a big fan of the new global variable names:
```
MaxChildren = MaxBackends + NUM_AUXILIARY_PROCS;
TotalProcs = MaxChildren + max_prepared_xacts;
TotalXactProcs = MaxBackends + max_prepared_xacts;
```

MaxChildren is actually the total number of backends plus auxiliary processes. Maybe something like MaxProcs would better reflect that?

TotalProcs is really the length of the PGPROC array in shared memory. Prepared transactions are not real processes; they just occupy some slots in the proc array. So perhaps a name like MaxPGProcSlots or MaxBackendAndPreparedSlots would be more accurate.

As for TotalXactProcs, maybe something like MaxBackendAndPreparedSlots would make the intent clearer.

Anyway, I am not very good at naming things. You or others may have better suggestions.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  13. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-27T08:00:18Z

    27.02.2026 10:48, Chao Li пишет:
> Anyway, I am not very good at naming things. You or others may have better suggestions.

I am bad at naming as well.
So I will not play this game.
Let committers do the way they like.

-- 
regards
Yura Sokolov aka funny-falcon

  14. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-02-27T10:58:21Z

    On 27/02/2026 07:00, Yura Sokolov wrote:
> 26.02.2026 23:44, Sami Imseih пишет:
>>> (Sorry, I've used Google Translate to write this sentence).
>>>
>>> When you write assert, you protect yourself from shooting your leg far in
>>> the future. Believe me.
>>
>> The issue to me seems that we a few code paths that rely on the
>> total proc calculation in several places, and changing one and
>> forgetting to change another is what broke things.
> 
> If there were asserts, then tests would have failed.
> Period.
> 
> There would no the bug. There would no this discussion.
> If only array bounds were checked with asserts.

Yeah, more asserts == good, usually.

Here's my version of this. It's the same basic idea, some minor changes:

- Instead of the MaxChildren, TotalProcs, TotalXactProcs variables, I 
added FIRST_PREPARED_XACT_PROC_NUMBER. Rationale: The variables still 
required you to know the layout of the PGPROCs, i.e. you still had to 
know that aux processes come after regular backends and dummy pgprocs 
after aux processes. An FIRST_PREPARED_XACT_PROC_NUMBER is more explicit 
in the places where it's needed.

   I have been thinking of adding variables like that anyway, because 
the current calculations with MaxBackends et al. are just so confusing. 
So I'm vaguely in favor of doing something like that in the future. But 
I think FIRST_PREPARED_XACT_PROC_NUMBER is more clear for this patch, 
and those variable names as done here (MaxBackends, MaxChildren, 
Totalprocs and TotalXactProcs) were still super confusing.

- I added a slightly different the set of Getter/Setter functions: 
MyOldestMemberMXactIdSlot(), 
PreparedXactOldestMemberMXactIdSlot(ProcNumber), and 
MyOldestVisibleMXactIdSlot(). I think this is slightly less error-prone, 
making it harder to pass proc number to wrong function (although the 
assertions would catch such misuses pretty quick anyway). And these 
functions return a pointer to the slot, so we don't need separate getter 
and setter functions. That's a matter of taste, having a little less 
code looks nicer to me.

What do you think?

I've been pondering what kind of issues this bug could cause. Because 
the OldestVisibleMXactId array is allocated just after the 
OldestMemberMXactId array, you don't get a buffer overflow, but a 
prepared xact can overwrite a backend's value in the 
OldestVisibleMXactId array. That can surely cause trouble later, but not 
sure what exactly, and it seems pretty hard to write a repro script for it.

- Heikki

  15. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-27T17:58:16Z

    27.02.2026 13:58, Heikki Linnakangas пишет:
> On 27/02/2026 07:00, Yura Sokolov wrote:
>> 26.02.2026 23:44, Sami Imseih пишет:
>>>> (Sorry, I've used Google Translate to write this sentence).
>>>>
>>>> When you write assert, you protect yourself from shooting your leg far in
>>>> the future. Believe me.
>>>
>>> The issue to me seems that we a few code paths that rely on the
>>> total proc calculation in several places, and changing one and
>>> forgetting to change another is what broke things.
>>
>> If there were asserts, then tests would have failed.
>> Period.
>>
>> There would no the bug. There would no this discussion.
>> If only array bounds were checked with asserts.
> 
> Yeah, more asserts == good, usually.
> 
> Here's my version of this. It's the same basic idea, some minor changes:
> 
> - Instead of the MaxChildren, TotalProcs, TotalXactProcs variables, I 
> added FIRST_PREPARED_XACT_PROC_NUMBER. Rationale: The variables still 
> required you to know the layout of the PGPROCs, i.e. you still had to 
> know that aux processes come after regular backends and dummy pgprocs 
> after aux processes. An FIRST_PREPARED_XACT_PROC_NUMBER is more explicit 
> in the places where it's needed.
> 
>    I have been thinking of adding variables like that anyway, because 
> the current calculations with MaxBackends et al. are just so confusing. 
> So I'm vaguely in favor of doing something like that in the future. But 
> I think FIRST_PREPARED_XACT_PROC_NUMBER is more clear for this patch, 
> and those variable names as done here (MaxBackends, MaxChildren, 
> Totalprocs and TotalXactProcs) were still super confusing.
> 
> - I added a slightly different the set of Getter/Setter functions: 
> MyOldestMemberMXactIdSlot(), 
> PreparedXactOldestMemberMXactIdSlot(ProcNumber), and 
> MyOldestVisibleMXactIdSlot(). I think this is slightly less error-prone, 
> making it harder to pass proc number to wrong function (although the 
> assertions would catch such misuses pretty quick anyway). And these 
> functions return a pointer to the slot, so we don't need separate getter 
> and setter functions. That's a matter of taste, having a little less 
> code looks nicer to me.
> 
> What do you think?

I stick to separate getters and setters because they are used in 64bit xid
patch.

With 32bit TransactionId PostgreSQL code in many places rely on 32bit
atomicity. But uint64 is not so atomic on currently-mostly-ancient
architecures. Therefore there is a need to use
pg_atomic_read_u64/pg_atomic_write_u64. And without hiding them in
getters/setters code become ugly.

So, getters+setters are better suited for (possible far) future.
But I don't mind if you stick with slot references for now.

(I realized now, to be prepared for uint64 getters have to be used in
GetOldestMultiXactId and MultiXactIdSetOldestVisible as well. My bad.)

> I've been pondering what kind of issues this bug could cause. Because 
> the OldestVisibleMXactId array is allocated just after the 
> OldestMemberMXactId array, you don't get a buffer overflow, but a 
> prepared xact can overwrite a backend's value in the 
> OldestVisibleMXactId array. That can surely cause trouble later, but not 
> sure what exactly, and it seems pretty hard to write a repro script for it.

With extremely low configured MaxBackends it will overflow to
BufferDescriptors array. Certainly only test configurations may use such
tiny settings.

I found the bug because I increased NUM_AUXILIARY_PROCS by bunch of other
specialized processes, so overflow to BufferDescriptors happened even with
parameters already set in tests.

-- 
regards
Yura Sokolov aka funny-falcon

  16. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-02-27T18:51:16Z

    >> There would no the bug. There would no this discussion.
>> If only array bounds were checked with asserts.

> Yeah, more asserts == good, usually.

Maybe. My thoughts were that If the MaxOldestSlot is set correctly at
initialization time,
and we guarantee that by asserting its size against the total number
of procs, that should
be sufficient, and we don't need to create wrapper functions or add
assertions on every
access. But. I don't have a strong opinion either way.

Anyhow, it looks like the proposals now are aiming towards eliminating
the need to account
for auxiliary procs, and that is probably OK. It's a small bit of
memory saved, but slightly bit
more code.

> With extremely low configured MaxBackends it will overflow to
> BufferDescriptors array. Certainly only test configurations may use such
> tiny settings.
>
> I found the bug because I increased NUM_AUXILIARY_PROCS by bunch of other
> specialized processes, so overflow to BufferDescriptors happened even with
> parameters already set in tests.

It's unfortunately a bit worse. Here is a repro that shows 2 prepared
transactions
being created after a shared lock is taken on a table with 1 row. A subsequent
delete is able to complete, where we would expect it to be blocked until the
prepared transactions COMMIT or ROLLBACK.

With simply adding NUM_AUXILIARY_PROCS to MaxOldestSlot, it works as expected,
and the delete is blocked.

```
-#define MaxOldestSlot  (MaxBackends + max_prepared_xacts)
+#define MaxOldestSlot  (MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts)
 ```

I also tried with Heikki's proposal and the test succeeded. I did not
review the patch thoroughly
yet, but I think this test case should be added.

The test does require 2 prepared transactions to exercise
MultiXactIdSetOldestVisible(),
which then results in reading garbage values from the array and
determining incorrect
visibility of the row.

--
Sami Imseih
Amazon Web Services (AWS)

  17. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-02-27T19:11:38Z

    > Here is a repro that shows

sorry, reattaching another version of the repro cleaned up
from remnants of an earlier renaming of the test.

--
Sami Imseih
Amazon Web Services (AWS)

  18. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-02-28T07:43:49Z

    27.02.2026 21:51, Sami Imseih пишет:
> It's unfortunately a bit worse. Here is a repro that shows 2 prepared
> transactions
> being created after a shared lock is taken on a table with 1 row. A subsequent
> delete is able to complete, where we would expect it to be blocked until the
> prepared transactions COMMIT or ROLLBACK.

OH MY GOD!!!!
I saw very similar behavior in dump of production WAL logs from our client.
Main issue were in other subsystem in that case, that is why we didn't dig
deeper.
But I clearly remembered: "Ouch, it is strange, why this row were deleted
being locked for foreign key?".

Great catch!!!!
My deep respect to you, Sami Imseih!!!!

> With simply adding NUM_AUXILIARY_PROCS to MaxOldestSlot, it works as expected,
> and the delete is blocked.
-- 
regards
Yura Sokolov aka funny-falcon

  19. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-02-28T09:22:03Z

    (added back pgsql-hackers, I think you replied in private by accident)

On 28/02/2026 00:07, Sami Imseih wrote:
> 1/
> 
> +MyOldestMemberMXactIdSlot(void)
> +{
> +       Assert(MyProcNumber >= 0 && MyProcNumber < MaxBackends);
> 
> It would be better to use NumVisibleSlots instead of MaxBackends in
> the second assert condition.

MaxBackends makes the assertion more strict. It verifies that we use one 
of the slots reserved for regular backends, not prepared xacts. I'll add 
a comment on that.

> 2/
> 
> +static inline MultiXactId *
> +PreparedXactOldestMemberMXactIdSlot(ProcNumber procno)
> +{
> +       Assert(procno >= FIRST_PREPARED_XACT_PROC_NUMBER);
> +       Assert(procno - FIRST_PREPARED_XACT_PROC_NUMBER <
> +              NumMemberSlots);
> +       return &OldestMemberMXactId[procno -
> +              FIRST_PREPARED_XACT_PROC_NUMBER];
> +}
> 
> given
> 
> +#define FIRST_PREPARED_XACT_PROC_NUMBER \
> +        (MaxBackends + NUM_AUXILIARY_PROCS)
> 
> let's say, MaxBackends = 100 and NUM_AUXILIARY_PROCS = 38, the
> first prepared transaction procno will be 138. The current math
> will return an index of 0 (138 - 138 = 0), but this corrupts
> backend slot 0. 

Oof, you're right.

> Should we not account for NumVisibleSlots to skip
> over the regular backend slots?
> 
> ```
> return &OldestMemberMXactId[NumVisibleSlots + (procno -
> FIRST_PREPARED_XACT_PROC_NUMBER)];
> ```

That's not quite right either. NumVisibleSlots has nothing to do with 
the OldestMemberMXactId array, MaxBackends is the right offset here. 
NumVisibleSlots == MaxBackends, but that's just a coincidence.

New version attached.

- Heikki

  20. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-03-01T14:20:25Z

    28.02.2026 12:22, Heikki Linnakangas пишет:
> New version attached.
LGTM

-- 
regards
Yura Sokolov

  21. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-03-02T02:34:13Z

    > New version attached.

The comment here:

+static inline MultiXactId *
+PreparedXactOldestMemberMXactIdSlot(ProcNumber procno)
+{
+       /*
+        * The entries with indexes >= MaxBackends in the OldestMemberMXactId
+        * array are reserved for regular backends.

should say "array are reserved for prepared transactions"

otherwise, LGTM.

--
Sami Imseih
Amazon Web Services (AWS)

  22. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Chao Li <li.evan.chao@gmail.com> — 2026-03-02T04:08:02Z

    
> On Mar 2, 2026, at 10:34, Sami Imseih <samimseih@gmail.com> wrote:
> 
>> New version attached.
> 
> The comment here:
> 
> +static inline MultiXactId *
> +PreparedXactOldestMemberMXactIdSlot(ProcNumber procno)
> +{
> +       /*
> +        * The entries with indexes >= MaxBackends in the OldestMemberMXactId
> +        * array are reserved for regular backends.
> 
> should say "array are reserved for prepared transactions"
> 

+1

And I saw the other code comment in multixact.c around line 162 needs an update because of this patch:
```
* Per-backend data starts here. We have two arrays stored in the area
* immediately following the MultiXactStateData struct. Each is indexed by
* ProcNumber.
```

It says “Each is indexed by ProcNumber”, but it’s no longer accurate for OldestMemberMXactId prepared-xact entries, which now use index (procno - FIRST_PREPARED_XACT_PROC_NUMBER).

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  23. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-03-02T17:24:02Z

    On 02/03/2026 06:08, Chao Li wrote:
>> The comment here:
>>
>> +static inline MultiXactId *
>> +PreparedXactOldestMemberMXactIdSlot(ProcNumber procno)
>> +{
>> +       /*
>> +        * The entries with indexes >= MaxBackends in the OldestMemberMXactId
>> +        * array are reserved for regular backends.
>>
>> should say "array are reserved for prepared transactions"
>>
> 
> +1
> 
> And I saw the other code comment in multixact.c around line 162 needs an update because of this patch:
> ```
> * Per-backend data starts here. We have two arrays stored in the area
> * immediately following the MultiXactStateData struct. Each is indexed by
> * ProcNumber.
> ```
> 
> It says “Each is indexed by ProcNumber”, but it’s no longer accurate for OldestMemberMXactId prepared-xact entries, which now use index (procno - FIRST_PREPARED_XACT_PROC_NUMBER).

Fixed those and some other comment work, and pushed. Thanks!

- Heikki

  24. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-03-02T19:09:07Z

    > > It says “Each is indexed by ProcNumber”, but it’s no longer accurate for OldestMemberMXactId prepared-xact entries, which now use index (procno - FIRST_PREPARED_XACT_PROC_NUMBER).
>
> Fixed those and some other comment work, and pushed. Thanks!
>

Thanks! what are your thoughts about adding a test like the one
here [1] ? This allows us to test correct handling of prepared
transaction dummy procs. The asserts added will not cover
this case.

What do you think?

[1] [https://www.postgresql.org/message-id/CAA5RZ0twq5bNMq0r0QNoopQnAEv%2BJ3qJNCrLs7HVqTEntBhJ%3Dg%40mail.gmail.com]

--
Sami Imseih
Amazon Web Services (AWS)

  25. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Yura Sokolov <y.sokolov@postgrespro.ru> — 2026-03-03T12:02:06Z

    02.03.2026 22:09, Sami Imseih пишет:
>>> It says “Each is indexed by ProcNumber”, but it’s no longer accurate for OldestMemberMXactId prepared-xact entries, which now use index (procno - FIRST_PREPARED_XACT_PROC_NUMBER).
>>
>> Fixed those and some other comment work, and pushed. Thanks!
>>
> 
> Thanks! what are your thoughts about adding a test like the one
> here [1] ? This allows us to test correct handling of prepared
> transaction dummy procs. The asserts added will not cover
> this case.
> 
> What do you think?
> 
> [1] [https://www.postgresql.org/message-id/CAA5RZ0twq5bNMq0r0QNoopQnAEv%2BJ3qJNCrLs7HVqTEntBhJ%3Dg%40mail.gmail.com]
I support: test for bug fixed is a good thing.

-- 
regards
Yura Sokolov aka funny-falcon

  26. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-03-03T15:11:05Z

    On 03/03/2026 14:02, Yura Sokolov wrote:
> 02.03.2026 22:09, Sami Imseih пишет:
>>>> It says “Each is indexed by ProcNumber”, but it’s no longer accurate for OldestMemberMXactId prepared-xact entries, which now use index (procno - FIRST_PREPARED_XACT_PROC_NUMBER).
>>>
>>> Fixed those and some other comment work, and pushed. Thanks!
>>>
>>
>> Thanks! what are your thoughts about adding a test like the one
>> here [1] ? This allows us to test correct handling of prepared
>> transaction dummy procs. The asserts added will not cover
>> this case.
>>
>> What do you think?
>>
>> [1] [https://www.postgresql.org/message-id/CAA5RZ0twq5bNMq0r0QNoopQnAEv%2BJ3qJNCrLs7HVqTEntBhJ%3Dg%40mail.gmail.com]
> I support: test for bug fixed is a good thing.

Ok, here's another version of Sami's repro. I realized that it doesn't 
even need concurrent sessions, so I moved it to the main regression test 
suite, into the 'prepared_xacts' test. Looks good?

- Heikki

  27. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-03T15:22:03Z

    Heikki Linnakangas <hlinnaka@iki.fi> writes:
> Ok, here's another version of Sami's repro. I realized that it doesn't 
> even need concurrent sessions, so I moved it to the main regression test 
> suite, into the 'prepared_xacts' test. Looks good?

Not directly relevant to the thing being tested here, but ...
I wonder if we couldn't reduce future maintenance effort for
prepared_xacts_1.out by modifying that test script to make an
early exit if prepared transactions are disabled.

			regards, tom lane

  28. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-03-03T16:59:34Z

    On 03/03/2026 17:22, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> Ok, here's another version of Sami's repro. I realized that it doesn't
>> even need concurrent sessions, so I moved it to the main regression test
>> suite, into the 'prepared_xacts' test. Looks good?
> 
> Not directly relevant to the thing being tested here, but ...
> I wonder if we couldn't reduce future maintenance effort for
> prepared_xacts_1.out by modifying that test script to make an
> early exit if prepared transactions are disabled.

+1, I was just thinking the same.

Here's a patch to do that. I'll backpatch this to all supported 
versions, to make backpatching test changes less painful in the future.

- Heikki

  29. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Sami Imseih <samimseih@gmail.com> — 2026-03-03T17:08:44Z

    > > Heikki Linnakangas <hlinnaka@iki.fi> writes:
> >> Ok, here's another version of Sami's repro. I realized that it doesn't
> >> even need concurrent sessions, so I moved it to the main regression test
> >> suite, into the 'prepared_xacts' test. Looks good?

You are correct. concurrent sessions not needed. v2 LGTM.

Thanks!

--
Sami Imseih
Amazon Web Services (AWS)

  30. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-03T18:47:57Z

    Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 03/03/2026 17:22, Tom Lane wrote:
>> I wonder if we couldn't reduce future maintenance effort for
>> prepared_xacts_1.out by modifying that test script to make an
>> early exit if prepared transactions are disabled.

> +1, I was just thinking the same.

Personally I'd put the exit test after the file's header comment,
but that's just a quibble.  LGTM otherwise.

			regards, tom lane

  31. Re: Fix bug in multixact Oldest*MXactId initialization and access

    Heikki Linnakangas <hlinnaka@iki.fi> — 2026-03-04T09:47:54Z

    On 03/03/2026 20:47, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> On 03/03/2026 17:22, Tom Lane wrote:
>>> I wonder if we couldn't reduce future maintenance effort for
>>> prepared_xacts_1.out by modifying that test script to make an
>>> early exit if prepared transactions are disabled.
> 
>> +1, I was just thinking the same.
> 
> Personally I'd put the exit test after the file's header comment,
> but that's just a quibble.  LGTM otherwise.

Yeah I agree it would look a little neater, but I put the exit test 
first so that the comment doesn't get echoed to the output when the test 
isn't run. That way we don't need to remember to update 
prepared_xacts_1.out if we change the header comment. It hasn't changed 
for 20 years, but still :-).

Committed, thanks!

- Heikki