| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
|---|---|
| To: | Sami Imseih <samimseih(at)gmail(dot)com> |
| Cc: | Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Subject: | Re: Fix bug in multixact Oldest*MXactId initialization and access |
| Date: | 2026-02-28 09:22:03 |
| Message-ID: | 231cd950-73e9-4db5-a516-c5216af7b93a@iki.fi |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
(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
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Fix-multixacts-OldestMemberMXactId-and-OldestVisi.patch | text/x-patch | 14.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | James Pang | 2026-02-28 09:46:30 | hot standby failed in read pg_commit_ts |
| Previous Message | Madhav Madhusoodanan | 2026-02-28 08:42:16 | Re: [WiP] B-tree page merge during vacuum to reduce index bloat |