Hi!
It seems my previous email was sent only to Andrey directly and didn't pass moderation
because it had a patch attached. I've now resent it from another email address.
----
In GetNewMultiXactId() this code may lead to error
---
ExtendMultiXactOffset(MultiXactState->nextMXact + 1);
---
If MultiXactState->nextMXact = MaxMultiXactId (0xFFFFFFFF)
we will not extend MultiXactOffset as we should
---
ExtendMultiXactOffset(0);
MultiXactIdToOffsetEntry(0)
multi % MULTIXACT_OFFSETS_PER_PAGE = 0
return; /* skip SLRU extension */
---
Perhaps we should introduce a simple function to handle next MultiXact
calculation
---
static inline MultiXactId
NextMultiXactId(MultiXactId multi)
{
return multi == MaxMultiXactId ? FirstMultiXactId : multi + 1;
}
---
I've attached a patch that fixes this issue, although it seems I've discovered
another overflow bug in multixact_redo().
We might call:
---
multixact_redo()
MultiXactAdvanceNextMXact(0xFFFFFFFF + 1, ...);
---
And if MultiXactState->nextMXact != InvalidMultiXactId (0), we will have
MultiXactState->nextMXact = 0.
This appears to cause problems in code that assumes MultiXactState->nextMXact
holds a valid MultiXactId.
For instance, in GetMultiXactId(), we would return an incorrect number
of MultiXacts.
Although, spreading MultiXact overflow handling throughout multixact.c code
seems error-prone.
Maybe we should use a macro instead (which would also allow us to modify this
check and add compiler hints):
---
#define MultiXactAdjustOverflow(mxact) \
if (unlikely((mxact) < FirstMultiXactId)) \
mxact = FirstMultiXactId;
---
