On 27/11/2025 05:39, Chao Li wrote:
>> <v11-0001-Avoid-multixact-edge-case-2-by-writing-the-next-.patch>
>
> 1
> ```
> + if (*offptr != offset)
> + {
> + /* should already be set to the correct value, or not at all */
> + Assert(*offptr == 0);
> + *offptr = offset;
> + MultiXactOffsetCtl->shared->page_dirty[slotno] = true;
> + }
> ```
>
> This is a more like a question. Since pre-write should always happen, in theory *offptr != offset should never be
true,why do we still need to handle the case instead of just assert(false)?
No, *offptr == 0 can happen, if multiple backends are generating
multixids concurrently. It's possible that we get here before the
RecordNewMultiXact() call for the previous multixid.
Similarly, the *next_offptr != 0 case can happen if another backend
calls RecordNewMultiXact() concurrently for the next multixid.
> 2
> ```
> + next = multi + 1;
> + if (next < FirstMultiXactId)
> + next = FirstMultiXactId;
> ```
>
> next < FirstMultiXactId will only be true when next wraps around to 0, maybe deserve one-line comment to explain
that.
This is a common pattern used in many places in the file.
> 3
> ```
> + if (*next_offptr != offset + nmembers)
> + {
> + /* should already be set to the correct value, or not at all */
> + Assert(*next_offptr == 0);
> + *next_offptr = offset + nmembers;
> + MultiXactMemberCtl->shared->page_dirty[slotno] = true;
> + }
> ```
>
> Should MultiXactMemberCtl be MultiXactOffsetCtl? As we are writing to the offset SLRU.
Good catch! Yes, it should be MultiXactOffsetCtl.
- Heikki
