Обсуждение: IPC/MultixactCreation on the Standby server

Поиск
Список
Период
Сортировка

IPC/MultixactCreation on the Standby server

От
Bykov Ivan
Дата:

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;

---

Вложения

Re: IPC/MultixactCreation on the Standby server

От
Kirill Reshke
Дата:
On Sat, 25 Oct 2025 at 18:59, Bykov Ivan <i.bykov@ftdata.ru> wrote:
>
> 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;
>
> ---


Hi! Are you sending patches which should be applied atop of [0] ?
There is no matching code in HEAD. If so, the better option in to post
to origin thread, fix while fixed patch

[0] https://www.postgresql.org/message-id/F5CF7FC1-955E-4E12-8A51-F4172B6977F2%40yandex-team.ru


-- 
Best regards,
Kirill Reshke