On 04/12/2025 16:36, Álvaro Herrera wrote:
> On 2025-Dec-04, Heikki Linnakangas wrote:
>
>> While working on the 64-bit multixid offsets patch, I noticed one more bug
>> with this. At offset wraparound, when we set the next multixid's offset in
>> RecordNewMultiXact, we incorrectly set it to 0 instead of 1. We're supposed
>> to skip over offset 1, because 0 is reserved to mean invalid. We do that
>> correctly when setting the "current" multixid's offset, because the caller
>> of RecordNewMultiXact has already skipped over offset 0, but I missed it for
>> the next offset.
>
> Ouch.
Committed the fix.
>> I tried to modify the new wraparound TAP test to reproduce that, but it
>> turned out to be difficult because you need to have multiple backends
>> assigning multixids concurrently to hit that.
>
> Hmm, would it make sense to add a pgbench-based test on
> src/test/modules/xid_wraparound? That module is already known to be
> expensive, and it doesn't run unless explicitly enabled, so I think it's
> not a bad fit.
Doesn't seem worth it. The repro with pgbench is a bit unreliable too.
It actually only readily reproduces on 'master'. In backbranches, I also
had to add a random sleep before RecordNewMultiXact() to trigger it. I
think that's because on 'master', I removed the special case in
GetMultiXactIdMembers() to use the in-memory nextOffset value instead of
reading the next offset from the SLRU page, when we're reading the last
assigned multixid.
Furthermore, we will hopefully get rid of offset wraparounds soon with
the 64-bit offsets.
- Heikki
