From 1bc2b558710aab2ee2c94c319b27587907cc3695 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 27 Feb 2026 12:30:24 +0200 Subject: [PATCH 1/1] 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" Author: Yura Sokolov Discussion: https://www.postgresql.org/message-id/7acc94b0-ea82-4657-b1b0-77842cb7a60c@postgrespro.ru --- src/backend/access/transam/multixact.c | 106 +++++++++++++++++-------- src/backend/access/transam/twophase.c | 7 +- src/backend/storage/lmgr/proc.c | 4 +- src/include/storage/proc.h | 1 + 4 files changed, 79 insertions(+), 39 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 90ec87d9dd6..bc45edb4c47 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 the 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 the 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. @@ -208,9 +211,10 @@ typedef struct MultiXactStateData } MultiXactStateData; /* - * Size of OldestMemberMXactId and OldestVisibleMXactId arrays. + * Sizes of OldestMemberMXactId and OldestVisibleMXactId arrays. */ -#define MaxOldestSlot (MaxBackends + max_prepared_xacts) +#define NumMemberSlots (MaxBackends + max_prepared_xacts) +#define NumVisibleSlots MaxBackends /* Pointers to the state data in shared memory */ static MultiXactStateData *MultiXactState; @@ -218,6 +222,28 @@ static MultiXactId *OldestMemberMXactId; static MultiXactId *OldestVisibleMXactId; +static inline MultiXactId * +MyOldestMemberMXactIdSlot(void) +{ + Assert(MyProcNumber >= 0 && MyProcNumber < MaxBackends); + return &OldestMemberMXactId[MyProcNumber]; +} + +static inline MultiXactId * +PreparedXactOldestMemberMXactIdSlot(ProcNumber procno) +{ + Assert(procno >= FIRST_PREPARED_XACT_PROC_NUMBER); + Assert(procno - FIRST_PREPARED_XACT_PROC_NUMBER < NumMemberSlots); + return &OldestMemberMXactId[procno - FIRST_PREPARED_XACT_PROC_NUMBER]; +} + +static inline MultiXactId * +MyOldestVisibleMXactIdSlot(void) +{ + Assert(MyProcNumber >= 0 && MyProcNumber < NumVisibleSlots); + return &OldestVisibleMXactId[MyProcNumber]; +} + /* * Definitions for the backend-local MultiXactId cache. * @@ -308,7 +334,7 @@ MultiXactIdCreate(TransactionId xid1, MultiXactStatus status1, Assert(!TransactionIdEquals(xid1, xid2) || (status1 != status2)); /* MultiXactIdSetOldestMember() must have been called already. */ - Assert(MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])); + Assert(MultiXactIdIsValid(*MyOldestMemberMXactIdSlot())); /* * Note: unlike MultiXactIdExpand, we don't bother to check that both XIDs @@ -362,7 +388,7 @@ MultiXactIdExpand(MultiXactId multi, TransactionId xid, MultiXactStatus status) Assert(TransactionIdIsValid(xid)); /* MultiXactIdSetOldestMember() must have been called already. */ - Assert(MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])); + Assert(MultiXactIdIsValid(*MyOldestMemberMXactIdSlot())); debug_elog5(DEBUG2, "Expand: received multi %u, xid %u status %s", multi, xid, mxstatus_to_string(status)); @@ -536,7 +562,7 @@ MultiXactIdIsRunning(MultiXactId multi, bool isLockOnly) void MultiXactIdSetOldestMember(void) { - if (!MultiXactIdIsValid(OldestMemberMXactId[MyProcNumber])) + if (!MultiXactIdIsValid(*MyOldestMemberMXactIdSlot())) { MultiXactId nextMXact; @@ -558,7 +584,7 @@ MultiXactIdSetOldestMember(void) nextMXact = MultiXactState->nextMXact; - OldestMemberMXactId[MyProcNumber] = nextMXact; + *MyOldestMemberMXactIdSlot() = nextMXact; LWLockRelease(MultiXactGenLock); @@ -586,7 +612,7 @@ MultiXactIdSetOldestMember(void) static void MultiXactIdSetOldestVisible(void) { - if (!MultiXactIdIsValid(OldestVisibleMXactId[MyProcNumber])) + if (!MultiXactIdIsValid(*MyOldestVisibleMXactIdSlot())) { MultiXactId oldestMXact; int i; @@ -594,7 +620,7 @@ MultiXactIdSetOldestVisible(void) LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); oldestMXact = MultiXactState->nextMXact; - for (i = 0; i < MaxOldestSlot; i++) + for (i = 0; i < NumMemberSlots; i++) { MultiXactId thisoldest = OldestMemberMXactId[i]; @@ -603,7 +629,7 @@ MultiXactIdSetOldestVisible(void) oldestMXact = thisoldest; } - OldestVisibleMXactId[MyProcNumber] = oldestMXact; + *MyOldestVisibleMXactIdSlot() = oldestMXact; LWLockRelease(MultiXactGenLock); @@ -1152,7 +1178,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members, * multi. It cannot possibly still be running. */ if (isLockOnly && - MultiXactIdPrecedes(multi, OldestVisibleMXactId[MyProcNumber])) + MultiXactIdPrecedes(multi, *MyOldestVisibleMXactIdSlot())) { debug_elog2(DEBUG2, "GetMembers: a locker-only multi is too old"); *members = NULL; @@ -1574,8 +1600,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; + *MyOldestMemberMXactIdSlot() = InvalidMultiXactId; + *MyOldestVisibleMXactIdSlot() = InvalidMultiXactId; /* * Discard the local MultiXactId cache. Since MXactContext was created as @@ -1595,7 +1621,7 @@ AtEOXact_MultiXact(void) void AtPrepare_MultiXact(void) { - MultiXactId myOldestMember = OldestMemberMXactId[MyProcNumber]; + MultiXactId myOldestMember = *MyOldestMemberMXactIdSlot(); if (MultiXactIdIsValid(myOldestMember)) RegisterTwoPhaseRecord(TWOPHASE_RM_MULTIXACT_ID, 0, @@ -1615,7 +1641,7 @@ PostPrepare_MultiXact(FullTransactionId fxid) * Transfer our OldestMemberMXactId value to the slot reserved for the * prepared transaction. */ - myOldestMember = OldestMemberMXactId[MyProcNumber]; + myOldestMember = *MyOldestMemberMXactIdSlot(); if (MultiXactIdIsValid(myOldestMember)) { ProcNumber dummyProcNumber = TwoPhaseGetDummyProcNumber(fxid, false); @@ -1628,8 +1654,8 @@ PostPrepare_MultiXact(FullTransactionId fxid) */ LWLockAcquire(MultiXactGenLock, LW_EXCLUSIVE); - OldestMemberMXactId[dummyProcNumber] = myOldestMember; - OldestMemberMXactId[MyProcNumber] = InvalidMultiXactId; + *PreparedXactOldestMemberMXactIdSlot(dummyProcNumber) = myOldestMember; + *MyOldestMemberMXactIdSlot() = InvalidMultiXactId; LWLockRelease(MultiXactGenLock); } @@ -1642,7 +1668,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; + *MyOldestVisibleMXactIdSlot() = InvalidMultiXactId; /* * Discard the local MultiXactId cache like in AtEOXact_MultiXact. @@ -1669,7 +1695,7 @@ multixact_twophase_recover(FullTransactionId fxid, uint16 info, Assert(len == sizeof(MultiXactId)); oldestMember = *((MultiXactId *) recdata); - OldestMemberMXactId[dummyProcNumber] = oldestMember; + *PreparedXactOldestMemberMXactIdSlot(dummyProcNumber) = oldestMember; } /* @@ -1684,7 +1710,7 @@ multixact_twophase_postcommit(FullTransactionId fxid, uint16 info, Assert(len == sizeof(MultiXactId)); - OldestMemberMXactId[dummyProcNumber] = InvalidMultiXactId; + *PreparedXactOldestMemberMXactIdSlot(dummyProcNumber) = InvalidMultiXactId; } /* @@ -1703,17 +1729,25 @@ multixact_twophase_postabort(FullTransactionId fxid, uint16 info, * thus double memory. Also, reserve space for the shared MultiXactState * struct and the per-backend MultiXactId arrays (two of those, too). */ +static Size +MultiXactSharedStateShmemSize() +{ + Size size; + + size = offsetof(MultiXactStateData, perBackendXactIds); + size = add_size(size, + mul_size(sizeof(MultiXactId), NumMemberSlots)); + size = add_size(size, + mul_size(sizeof(MultiXactId), NumVisibleSlots)); + return size; +} + Size 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)) - - size = SHARED_MULTIXACT_STATE_SIZE; + size = MultiXactSharedStateShmemSize(); size = add_size(size, SimpleLruShmemSize(multixact_offset_buffers, 0)); size = add_size(size, SimpleLruShmemSize(multixact_member_buffers, 0)); @@ -1747,14 +1781,14 @@ MultiXactShmemInit(void) /* Initialize our shared state struct */ MultiXactState = ShmemInitStruct("Shared MultiXact State", - SHARED_MULTIXACT_STATE_SIZE, + MultiXactSharedStateShmemSize(), &found); if (!IsUnderPostmaster) { Assert(!found); /* Make sure we zero out the per-backend state */ - MemSet(MultiXactState, 0, SHARED_MULTIXACT_STATE_SIZE); + MemSet(MultiXactState, 0, MultiXactSharedStateShmemSize()); } else Assert(found); @@ -1763,7 +1797,7 @@ MultiXactShmemInit(void) * Set up array pointers. */ OldestMemberMXactId = MultiXactState->perBackendXactIds; - OldestVisibleMXactId = OldestMemberMXactId + MaxOldestSlot; + OldestVisibleMXactId = OldestMemberMXactId + NumMemberSlots; } /* @@ -2303,7 +2337,6 @@ MultiXactId GetOldestMultiXactId(void) { MultiXactId oldestMXact; - int i; /* * This is the oldest valid value among all the OldestMemberMXactId[] and @@ -2311,7 +2344,7 @@ GetOldestMultiXactId(void) */ LWLockAcquire(MultiXactGenLock, LW_SHARED); oldestMXact = MultiXactState->nextMXact; - for (i = 0; i < MaxOldestSlot; i++) + for (int i = 0; i < NumMemberSlots; i++) { MultiXactId thisoldest; @@ -2319,6 +2352,11 @@ GetOldestMultiXactId(void) if (MultiXactIdIsValid(thisoldest) && MultiXactIdPrecedes(thisoldest, oldestMXact)) oldestMXact = thisoldest; + } + for (int i = 0; i < NumVisibleSlots; i++) + { + MultiXactId thisoldest; + thisoldest = OldestVisibleMXactId[i]; if (MultiXactIdIsValid(thisoldest) && MultiXactIdPrecedes(thisoldest, oldestMXact)) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index e4340b59640..5f65fa7b80f 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -900,9 +900,10 @@ TwoPhaseGetXidByVirtualXID(VirtualTransactionId vxid, * Get the dummy proc number for prepared transaction * * Dummy proc numbers are similar to proc numbers of real backends. They - * start at MaxBackends, and are unique across all currently active real - * backends and prepared transactions. If lock_held is set to true, - * TwoPhaseStateLock will not be taken, so the caller had better hold it. + * start at FIRST_PREPARED_XACT_PROC_NUMBER, and are unique across all + * currently active real backends and prepared transactions. If lock_held is + * set to true, TwoPhaseStateLock will not be taken, so the caller had better + * hold it. */ ProcNumber TwoPhaseGetDummyProcNumber(FullTransactionId fxid, bool lock_held) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 771b006b522..ccf9de0e67c 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -304,7 +304,7 @@ InitProcGlobal(void) * dummy PGPROCs don't need these though - they're never associated * with a real process */ - if (i < MaxBackends + NUM_AUXILIARY_PROCS) + if (i < FIRST_PREPARED_XACT_PROC_NUMBER) { proc->sem = PGSemaphoreCreate(); InitSharedLatch(&(proc->procLatch)); @@ -369,7 +369,7 @@ InitProcGlobal(void) * processes and prepared transactions. */ AuxiliaryProcs = &procs[MaxBackends]; - PreparedXactProcs = &procs[MaxBackends + NUM_AUXILIARY_PROCS]; + PreparedXactProcs = &procs[FIRST_PREPARED_XACT_PROC_NUMBER]; } /* diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index a8d2e7db1a1..3f89450c216 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -532,6 +532,7 @@ extern PGDLLIMPORT PGPROC *PreparedXactProcs; #define MAX_IO_WORKERS 32 #define NUM_AUXILIARY_PROCS (6 + MAX_IO_WORKERS) +#define FIRST_PREPARED_XACT_PROC_NUMBER (MaxBackends + NUM_AUXILIARY_PROCS) /* configurable options */ extern PGDLLIMPORT int DeadlockTimeout; -- 2.47.3