pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Andres Freund <andres@anarazel.de>
Cc: Chao Li <li.evan.chao@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>
Subject: Re: Fix bug in multixact Oldest*MXactId initialization and access
Date: Wed, 25 Feb 2026 18:16:27 +0300
Message-ID: <c3eaaac1-d244-4e4d-b5d7-06c284108eac@postgrespro.ru> (raw)
In-Reply-To: <93F0BD0A-0B8B-4592-9D4B-53F405A86F6A@gmail.com>
References: <7acc94b0-ea82-4657-b1b0-77842cb7a60c@postgrespro.ru>
	<93F0BD0A-0B8B-4592-9D4B-53F405A86F6A@gmail.com>

Good day.

Chao Li and Sami Imseih, thank you for looking at.

After thinking a bit, I've decided to make sizes of arrays precise:
- OldestMemberMXactId's size remains MaxBackends + max_prepared_xacts.
  Instead of changing its size, procno is now adjusted to not include
auxiliary procs.
- OldestVisibleMXactId contains only MaxBackends elemenents now.
  It is used only for real backends and not prepared transactions.

All accesses are validated with asserts certainly.

I believe, index transformation in access of OldestMemberMXactId will not
cost much since all this operations are quite rare.
In the loops arrays are accessed directly since limiting loop index is enough.

-- 
regards
Yura Sokolov aka funny-falcon

Attachments:

v3-0001-Fix-multixacts-OldestMemberMXactId-and-OldestVisi.patch (text/x-patch, 9.8K) download From c4ae9ee5f8d287cbda8c4a5a780ad6ca25b1ede1 Mon Sep 17 00:00:00 2001 From: Yura Sokolov <y.sokolov@postgrespro.ru> Date: Wed, 25 Feb 2026 18:03:57 +0300 Subject: [PATCH v3] Fix multixacts OldestMemberMXactId and OldestVisibleMXactId usage Due to [1], OldestMemberMXactId is no longer accessed by synthetic dummyBackendId, but rather with pgprocno. Procs for prepared xacts are placed after auxiliary procs, therefore calculation for MaxOldestSlot became invalid. On the other hand, OldestVisibleMXactId is used only for real backends, and so never accessed at index greater than MaxBackends. Lets separate size calculation for arrays and use conversion of index in access to OldestMemberMXactId. [1] ab355e3a88de745 "Redefine backend ID to be an index into the proc array" --- src/backend/access/transam/multixact.c | 102 +++++++++++++++++-------- 1 file changed, 70 insertions(+), 32 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 90ec87d9dd6..ca515092cfd 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -159,12 +159,7 @@ typedef struct MultiXactStateData /* * Per-backend data starts here. We have two arrays stored in the area - * immediately following the MultiXactStateData struct. Each is indexed by - * ProcNumber. - * - * In both arrays, there's a slot for all normal backends - * (0..MaxBackends-1) followed by a slot for max_prepared_xacts prepared - * transactions. + * immediately following the MultiXactStateData struct. * * OldestMemberMXactId[k] is the oldest MultiXactId each backend's current * transaction(s) could possibly be a member of, or InvalidMultiXactId @@ -203,6 +198,14 @@ typedef struct MultiXactStateData * In turn, the minimum of all of those values is stored in pg_control. * This is used as the truncation point for pg_multixact when unneeded * segments get removed by vac_truncate_clog() during vacuuming. + * + * In OldestMemberMXactId[] there's a slot for all normal backends + * (0..MaxBackends-1) followed by a slot for max_prepared_xacts prepared + * transactions. It is indexed by value derived from ProcNumber. + * + * OldestVisibleMXactId has only slots for normal backends since it is not + * used for prepared transactions. And it is indexed by ProcNumber + * directly. */ MultiXactId perBackendXactIds[FLEXIBLE_ARRAY_MEMBER]; } MultiXactStateData; @@ -210,13 +213,52 @@ typedef struct MultiXactStateData /* * Size of OldestMemberMXactId and OldestVisibleMXactId arrays. */ -#define MaxOldestSlot (MaxBackends + max_prepared_xacts) +#define MaxOldestMemberSlot (MaxBackends + max_prepared_xacts) +#define MaxOldestVisibleSlot MaxBackends +#define MaxOldestSlot (MaxOldestMemberSlot + MaxOldestVisibleSlot) /* Pointers to the state data in shared memory */ static MultiXactStateData *MultiXactState; static MultiXactId *OldestMemberMXactId; static MultiXactId *OldestVisibleMXactId; +static inline MultiXactId +GetOldestMemberMXactId(ProcNumber procno) +{ + if (procno >= MaxBackends) + { + Assert(procno >= MaxBackends + NUM_AUXILIARY_PROCS); + procno -= NUM_AUXILIARY_PROCS; + } + Assert(procno >= 0 && procno < MaxOldestMemberSlot); + return OldestMemberMXactId[procno]; +} + +static inline void +SetOldestMemberMXactId(ProcNumber procno, MultiXactId value) +{ + if (procno >= MaxBackends) + { + Assert(procno >= MaxBackends + NUM_AUXILIARY_PROCS); + procno -= NUM_AUXILIARY_PROCS; + } + Assert(procno >= 0 && procno < MaxOldestMemberSlot); + OldestMemberMXactId[procno] = value; +} + +static inline MultiXactId +GetOldestVisibleMXactId(ProcNumber procno) +{ + Assert(procno >= 0 && procno < MaxOldestVisibleSlot); + return OldestVisibleMXactId[procno]; +} + +static inline void +SetOldestVisibleMXactId(ProcNumber procno, MultiXactId value) +{ + Assert(procno >= 0 && procno < MaxOldestVisibleSlot); + OldestVisibleMXactId[procno] = value; +} /* * Definitions for the backend-local MultiXactId cache. @@ -308,7 +350,7 @@ MultiXactIdCreate(TransactionId xid1, MultiXactStatus status1, Assert(!TransactionIdEquals(xid1, xid2) || (status1 != status2)); /* MultiXactIdSetOldestMember() must have been called already. */ - Assert(MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])); + Assert(MultiXactIdIsValid(GetOldestMemberMXactId(MyProcNumber))); /* * Note: unlike MultiXactIdExpand, we don't bother to check that both XIDs @@ -362,7 +404,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status) Assert(TransactionIdIsValid(xid)); /* MultiXactIdSetOldestMember() must have been called already. */ - Assert(MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])); + Assert(MultiXactIdIsValid(GetOldestMemberMXactId(MyProcNumber))); debug_elog5(DEBUG2, "Expand: received multi %u, xid %u status %s", multi, xid, mxstatus_to_string(status)); @@ -536,7 +578,7 @@ MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly) void MultiXactIdSetOldestMember(void) { - if (!MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])) + if (!MultiXactIdIsValid(GetOldestMemberMXactId(MyProcNumber))) { MultiXactId nextMXact; @@ -558,7 +600,7 @@ MultiXactIdSetOldestMember(void) nextMXact = MultiXactState->nextMXact; - OldestMemberMXactId[MyProcNumber] = nextMXact; + SetOldestMemberMXactId(MyProcNumber, nextMXact); LWLockRelease(MultiXactGenLock); @@ -594,7 +636,7 @@ MultiXactIdSetOldestVisible(void) LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); oldestMXact = MultiXactState->nextMXact; - for (i = 0; i < MaxOldestSlot; i++) + for (i = 0; i < MaxOldestMemberSlot; i++) { MultiXactId thisoldest = OldestMemberMXactId[i]; @@ -603,7 +645,7 @@ MultiXactIdSetOldestVisible(void) oldestMXact = thisoldest; } - OldestVisibleMXactId[MyProcNumber] = oldestMXact; + SetOldestVisibleMXactId(MyProcNumber, oldestMXact); LWLockRelease(MultiXactGenLock); @@ -1574,8 +1616,8 @@ AtEOXact_MultiXact(void) * We assume that storing a MultiXactId is atomic and so we need not take * MultiXactGenLock to do this. */ - OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId; - OldestVisibleMXactId[MyProcNumber] = InvalidMultiXactId; + SetOldestMemberMXactId(MyProcNumber, InvalidMultiXactId); + SetOldestVisibleMXactId(MyProcNumber, InvalidMultiXactId); /* * Discard the local MultiXactId cache. Since MXactContext was created as @@ -1595,7 +1637,7 @@ AtEOXact_MultiXact(void) void AtPrepare_MultiXact(void) { - MultiXactId myOldestMember = OldestMemberMXactId[MyProcNumber]; + MultiXactId myOldestMember = GetOldestMemberMXactId(MyProcNumber); if (MultiXactIdIsValid(myOldestMember)) RegisterTwoPhaseRecord(TWOPHASE_RM_MULTIXACT_ID, 0, @@ -1615,7 +1657,7 @@ PostPrepare_MultiXact(FullTransactionId fxid) * Transfer our OldestMemberMXactId value to the slot reserved for the * prepared transaction. */ - myOldestMember = OldestMemberMXactId[MyProcNumber]; + myOldestMember = GetOldestMemberMXactId(MyProcNumber); if (MultiXactIdIsValid(myOldestMember)) { ProcNumber dummyProcNumber = TwoPhaseGetDummyProcNumber(fxid, false); @@ -1628,8 +1670,8 @@ PostPrepare_MultiXact(FullTransactionId fxid) */ LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); - OldestMemberMXactId[dummyProcNumber] = myOldestMember; - OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId; + SetOldestMemberMXactId(dummyProcNumber, myOldestMember); + SetOldestMemberMXactId(MyProcNumber, InvalidMultiXactId); LWLockRelease(MultiXactGenLock); } @@ -1642,7 +1684,7 @@ PostPrepare_MultiXact(FullTransactionId fxid) * We assume that storing a MultiXactId is atomic and so we need not take * MultiXactGenLock to do this. */ - OldestVisibleMXactId[MyProcNumber] = InvalidMultiXactId; + SetOldestVisibleMXactId(MyProcNumber, InvalidMultiXactId); /* * Discard the local MultiXactId cache like in AtEOXact_MultiXact. @@ -1669,7 +1711,7 @@ multixact_twophase_recover(FullTransactionId fxid, uint16 info, Assert(len == sizeof(MultiXactId)); oldestMember = *((MultiXactId *) recdata); - OldestMemberMXactId[dummyProcNumber] = oldestMember; + SetOldestMemberMXactId(dummyProcNumber, oldestMember); } /* @@ -1684,7 +1726,7 @@ multixact_twophase_postcommit(FullTransactionId fxid, uint16 info, Assert(len == sizeof(MultiXactId)); - OldestMemberMXactId[dummyProcNumber] = InvalidMultiXactId; + SetOldestMemberMXactId(dummyProcNumber, InvalidMultiXactId); } /* @@ -1708,10 +1750,9 @@ MultiXactShmemSize(void) { Size size; - /* We need 2*MaxOldestSlot perBackendXactIds[] entries */ #define SHARED_MULTIXACT_STATE_SIZE \ add_size(offsetof(MultiXactStateData, perBackendXactIds), \ - mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot)) + mul_size(sizeof(MultiXactId), MaxOldestSlot)) size = SHARED_MULTIXACT_STATE_SIZE; size = add_size(size, SimpleLruShmemSize(multixact_offset_buffers, 0)); @@ -1763,7 +1804,7 @@ MultiXactShmemInit(void) * Set up array pointers. */ OldestMemberMXactId = MultiXactState->perBackendXactIds; - OldestVisibleMXactId = OldestMemberMXactId + MaxOldestSlot; + OldestVisibleMXactId = OldestMemberMXactId + MaxOldestMemberSlot; } /* @@ -2303,6 +2344,8 @@ MultiXactId GetOldestMultiXactId(void) { MultiXactId oldestMXact; + MultiXactId thisoldest; + MultiXactId *allOldest; int i; /* @@ -2311,15 +2354,10 @@ GetOldestMultiXactId(void) */ LWLockAcquire(MultiXactGenLock, LW_SHARED); oldestMXact = MultiXactState->nextMXact; + allOldest = MultiXactState->perBackendXactIds; for (i = 0; i < MaxOldestSlot; i++) { - MultiXactId thisoldest; - - thisoldest = OldestMemberMXactId[i]; - if (MultiXactIdIsValid(thisoldest) && - MultiXactIdPrecedes(thisoldest, oldestMXact)) - oldestMXact = thisoldest; - thisoldest = OldestVisibleMXactId[i]; + thisoldest = allOldest[i]; if (MultiXactIdIsValid(thisoldest) && MultiXactIdPrecedes(thisoldest, oldestMXact)) oldestMXact = thisoldest; -- 2.51.0

view thread (31+ messages)  latest in thread

Message-ID: <c3eaaac1-d244-4e4d-b5d7-06c284108eac@postgrespro.ru>
Permalink:  ../c3eaaac1-d244-4e4d-b5d7-06c284108eac@postgrespro.ru/
Also on:    postgresql.org/message-id/c3eaaac1-d244-4e4d-b5d7-06c284108eac@postgrespro.ru

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: y.sokolov@postgrespro.ru, pgsql-hackers@lists.postgresql.org, hlinnaka@iki.fi, andres@anarazel.de, li.evan.chao@gmail.com, samimseih@gmail.com
  Subject: Re: Fix bug in multixact Oldest*MXactId initialization and access
  In-Reply-To: <c3eaaac1-d244-4e4d-b5d7-06c284108eac@postgrespro.ru>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox