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



Reply via email to