Re: Fix bug in multixact Oldest*MXactId initialization and access
| От | Chao Li |
|---|---|
| Тема | Re: Fix bug in multixact Oldest*MXactId initialization and access |
| Дата | |
| Msg-id | 93F0BD0A-0B8B-4592-9D4B-53F405A86F6A@gmail.com обсуждение |
| Ответ на | Fix bug in multixact Oldest*MXactId initialization and access (Yura Sokolov <y.sokolov@postgrespro.ru>) |
| Ответы |
Re: Fix bug in multixact Oldest*MXactId initialization and access
|
| Список | pgsql-hackers |
> 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 isunchanged from the original v00. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Вложения
В списке pgsql-hackers по дате отправления: