Re: Fix bug in multixact Oldest*MXactId initialization and access
Sami Imseih <samimseih@gmail.com>
From: Sami Imseih <samimseih@gmail.com>
To: Yura Sokolov <y.sokolov@postgrespro.ru>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Heikki Linnakangas <hlinnaka@iki.fi>, Andres Freund <andres@anarazel.de>,
Chao Li <li.evan.chao@gmail.com>
Date: 2026-02-27T18:51:16Z
Lists: pgsql-hackers
Attachments
- v1-0001-repro.patch (application/x-patch) patch v1-0001
>> 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)
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Add test for row-locking and multixids with prepared transactions
- 969576dab47b 17.10 landed
- fa3b328e6dc5 18.4 landed
- fe08113aefc9 19 (unreleased) landed
-
Skip prepared_xacts test if max_prepared_transactions < 2
- ffe53037dff7 14.23 landed
- a6b11ac4c42d 15.18 landed
- 65f5c451a3e1 16.14 landed
- 80d1d4ee06fd 17.10 landed
- 201436c19f40 18.4 landed
- 19615a44b3e6 19 (unreleased) landed
-
Fix OldestMemberMXactId and OldestVisibleMXactId array usage
- dcd9c06a4203 17.10 landed
- 0a50ef094382 18.4 landed
- ccae90abdbb6 19 (unreleased) landed
-
Redefine backend ID to be an index into the proc array
- ab355e3a88de 17.0 cited
-
Fix an old bug in multixact and two-phase commit. Prepared transactions can
- cd87b6f8a508 9.0.0 cited