From d3c2bb72de0b4f5e3149cd8f01493c37315067ce Mon Sep 17 00:00:00 2001 From: Yura Sokolov Date: Thu, 26 Feb 2026 19:08:57 +0300 Subject: [PATCH v4 2/2] 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 converted index to store oldest member mxact for prepared transactions. [1] ab355e3a88de745 "Redefine backend ID to be an index into the proc array" --- src/backend/access/transam/multixact.c | 104 +++++++++++++++++-------- 1 file changed, 72 insertions(+), 32 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index a35671ea346..42859828513 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -162,10 +162,6 @@ typedef struct MultiXactStateData * 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. - * * OldestMemberMXactId[k] is the oldest MultiXactId each backend's current * transaction(s) could possibly be a member of, or InvalidMultiXactId * when the backend has no live transaction that could possibly be a @@ -176,6 +172,10 @@ typedef struct MultiXactStateData * member of a MultiXact, and that MultiXact would have to be created * during or after the lock acquisition.) * + * In OldestMemberMXactId array, there's a slot for all normal backends + * (0..MaxBackends-1) followed by a slot for max_prepared_xacts prepared + * transactions. + * * OldestVisibleMXactId[k] is the oldest MultiXactId each backend's * current transaction(s) think is potentially live, or InvalidMultiXactId * when not in a transaction or not in a transaction that's paid any @@ -187,6 +187,9 @@ typedef struct MultiXactStateData * than its own OldestVisibleMXactId[] setting; this is necessary because * the relevant SLRU data can be concurrently truncated away. * + * In OldestVisibleMXactId array, there's a slot for all normal backends + * (0..MaxBackends-1) only. No slots for prepared transactions. + * * The oldest valid value among all of the OldestMemberMXactId[] and * OldestVisibleMXactId[] entries is considered by vacuum as the earliest * possible value still having any live member transaction -- OldestMxact. @@ -210,13 +213,54 @@ typedef struct MultiXactStateData /* * Size of OldestMemberMXactId and OldestVisibleMXactId arrays. */ -#define MaxOldestSlot TotalXactProcs +#define MaxMemberSlot TotalXactProcs +#define MaxVisibleSlot MaxBackends +/* + * Total size of perBackendXactIds. + */ +#define MaxOldestSlot (MaxMemberSlot + MaxVisibleSlot) /* Pointers to the state data in shared memory */ static MultiXactStateData *MultiXactState; static MultiXactId *OldestMemberMXactId; static MultiXactId *OldestVisibleMXactId; +static inline MultiXactId +GetOldestMemberMXactId(ProcNumber procno) +{ + Assert(procno >= 0 && procno < MaxBackends); + return OldestMemberMXactId[procno]; +} + +static inline void +SetOldestMemberMXactId(ProcNumber procno, MultiXactId mxact) +{ + Assert(procno >= 0 && procno < MaxBackends); + OldestMemberMXactId[procno] = mxact; +} + +static inline void +SetOldestPreparedMXactId(ProcNumber procno, MultiXactId mxact) +{ + Assert(procno >= MaxChildren && procno < TotalProcs); + /* This check looks redundant, but it is future proof. */ + Assert(procno - MaxChildren + MaxBackends < MaxMemberSlot); + OldestMemberMXactId[procno - MaxChildren + MaxBackends] = mxact; +} + +static inline MultiXactId +GetOldestVisibleMXactId(ProcNumber procno) +{ + Assert(procno >= 0 && procno < MaxVisibleSlot); + return OldestVisibleMXactId[procno]; +} + +static inline void +SetOldestVisibleMXactId(ProcNumber procno, MultiXactId mxact) +{ + Assert(procno >= 0 && procno < MaxVisibleSlot); + OldestVisibleMXactId[procno] = mxact; +} /* * Definitions for the backend-local MultiXactId cache. @@ -308,7 +352,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 +406,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 +580,7 @@ MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly) void MultiXactIdSetOldestMember(void) { - if (!MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])) + if (!MultiXactIdIsValid(GetOldestMemberMXactId(MyProcNumber))) { MultiXactId nextMXact; @@ -558,7 +602,7 @@ MultiXactIdSetOldestMember(void) nextMXact = MultiXactState->nextMXact; - OldestMemberMXactId[MyProcNumber] = nextMXact; + SetOldestMemberMXactId(MyProcNumber, nextMXact); LWLockRelease(MultiXactGenLock); @@ -586,7 +630,7 @@ MultiXactIdSetOldestMember(void) static void MultiXactIdSetOldestVisible(void) { - if (!MultiXactIdIsValid(OldestVisibleMXactId[MyProcNumber])) + if (!MultiXactIdIsValid(GetOldestVisibleMXactId(MyProcNumber))) { MultiXactId oldestMXact; int i; @@ -594,7 +638,7 @@ MultiXactIdSetOldestVisible(void) LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); oldestMXact = MultiXactState->nextMXact; - for (i = 0; i < MaxOldestSlot; i++) + for (i = 0; i < MaxMemberSlot; i++) { MultiXactId thisoldest = OldestMemberMXactId[i]; @@ -603,7 +647,7 @@ MultiXactIdSetOldestVisible(void) oldestMXact = thisoldest; } - OldestVisibleMXactId[MyProcNumber] = oldestMXact; + SetOldestVisibleMXactId(MyProcNumber, oldestMXact); LWLockRelease(MultiXactGenLock); @@ -1152,7 +1196,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, * multi. It cannot possibly still be running. */ if (isLockOnly && - MultiXactIdPrecedes(multi, OldestVisibleMXactId[MyProcNumber])) + MultiXactIdPrecedes(multi, GetOldestVisibleMXactId(MyProcNumber))) { debug_elog2(DEBUG2, "GetMembers: a locker-only multi is too old"); *members = NULL; @@ -1574,8 +1618,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 +1639,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 +1659,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 +1672,8 @@ PostPrepare_MultiXact(FullTransactionId fxid) */ LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); - OldestMemberMXactId[dummyProcNumber] = myOldestMember; - OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId; + SetOldestPreparedMXactId(dummyProcNumber, myOldestMember); + SetOldestMemberMXactId(MyProcNumber, InvalidMultiXactId); LWLockRelease(MultiXactGenLock); } @@ -1642,7 +1686,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 +1713,7 @@ multixact_twophase_recover(FullTransactionId fxid, uint16 info, Assert(len == sizeof(MultiXactId)); oldestMember = *((MultiXactId *) recdata); - OldestMemberMXactId[dummyProcNumber] = oldestMember; + SetOldestPreparedMXactId(dummyProcNumber, oldestMember); } /* @@ -1684,7 +1728,7 @@ multixact_twophase_postcommit(FullTransactionId fxid, uint16 info, Assert(len == sizeof(MultiXactId)); - OldestMemberMXactId[dummyProcNumber] = InvalidMultiXactId; + SetOldestPreparedMXactId(dummyProcNumber, InvalidMultiXactId); } /* @@ -1708,10 +1752,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 +1806,7 @@ MultiXactShmemInit(void) * Set up array pointers. */ OldestMemberMXactId = MultiXactState->perBackendXactIds; - OldestVisibleMXactId = OldestMemberMXactId + MaxOldestSlot; + OldestVisibleMXactId = OldestMemberMXactId + MaxMemberSlot; } /* @@ -2303,6 +2346,8 @@ MultiXactId GetOldestMultiXactId(void) { MultiXactId oldestMXact; + MultiXactId thisoldest; + MultiXactId *allOldest; int i; /* @@ -2311,15 +2356,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