From ccf17928fb28853d9ac3af9c1c8155ff4bc9ff68 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 6 Apr 2020 21:28:55 -0700 Subject: [PATCH v7 08/11] snapshot scalability: Move PGXACT->vacuumFlags to ProcGlobal->vacuumFlags. Similar to the previous commit this increases the chance that data frequently needed by GetSnapshotData() stays in l2 cache. As we now take care to not unnecessarily write to ProcGlobal->vacuumFlags, there should be very few modifications to the ProcGlobal->vacuumFlags array. --- src/include/storage/proc.h | 12 ++++- src/backend/access/transam/twophase.c | 2 +- src/backend/commands/analyze.c | 10 ++-- src/backend/commands/vacuum.c | 5 +- src/backend/postmaster/autovacuum.c | 6 +-- src/backend/replication/logical/logical.c | 3 +- src/backend/replication/slot.c | 3 +- src/backend/storage/ipc/procarray.c | 57 +++++++++++++++-------- src/backend/storage/lmgr/deadlock.c | 6 +-- src/backend/storage/lmgr/proc.c | 16 ++++--- 10 files changed, 77 insertions(+), 43 deletions(-) diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 60586e8be34..37208ddf342 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -41,7 +41,7 @@ struct XidCache }; /* - * Flags for PGXACT->vacuumFlags + * Flags for ProcGlobal->vacuumFlags[] */ #define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */ #define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */ @@ -171,6 +171,10 @@ struct PGPROC struct XidCache subxids; /* cache for subtransaction XIDs */ + uint8 vacuumFlagsCopy; /* this backend's vacuum flags, a copy of its + * ProcGlobal->vacuumFlagsCopy[], see + * PROC_* above */ + /* Support for group XID clearing. */ /* true, if member of ProcArray group waiting for XID clear */ bool procArrayGroupMember; @@ -231,7 +235,6 @@ extern PGDLLIMPORT struct PGXACT *MyPgXact; */ typedef struct PGXACT { - uint8 vacuumFlags; /* vacuum-related flags, see above */ bool overflowed; uint8 nxids; @@ -276,6 +279,11 @@ typedef struct PROC_HDR */ TransactionId *xids; + /* + * Vacuum flags. See PROC_* above. + */ + uint8 *vacuumFlags; + /* Length of allProcs array */ uint32 allProcCount; /* Head of list of free PGPROC structures */ diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 8103c5cb71f..06b61605649 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -466,7 +466,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, proc->xidCopy = xid; Assert(proc->xmin == InvalidTransactionId); proc->delayChkpt = false; - pgxact->vacuumFlags = 0; + proc->vacuumFlagsCopy = 0; proc->pid = 0; proc->backendId = InvalidBackendId; proc->databaseId = databaseid; diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 7b75945c4a9..d55477464e9 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -250,7 +250,8 @@ analyze_rel(Oid relid, RangeVar *relation, * OK, let's do it. First let other backends know I'm in ANALYZE. */ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); - MyPgXact->vacuumFlags |= PROC_IN_ANALYZE; + MyProc->vacuumFlagsCopy |= PROC_IN_ANALYZE; + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy; LWLockRelease(ProcArrayLock); pgstat_progress_start_command(PROGRESS_COMMAND_ANALYZE, RelationGetRelid(onerel)); @@ -281,11 +282,12 @@ analyze_rel(Oid relid, RangeVar *relation, pgstat_progress_end_command(); /* - * Reset my PGXACT flag. Note: we need this here, and not in vacuum_rel, - * because the vacuum flag is cleared by the end-of-xact code. + * Reset vacuumFlags we set early. Note: we need this here, and not in + * vacuum_rel, because the vacuum flag is cleared by the end-of-xact code. */ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); - MyPgXact->vacuumFlags &= ~PROC_IN_ANALYZE; + MyProc->vacuumFlagsCopy &= ~PROC_IN_ANALYZE; + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy; LWLockRelease(ProcArrayLock); } diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 1cc220c2d56..1da7b4d3e06 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1730,9 +1730,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) * might appear to go backwards, which is probably Not Good. */ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); - MyPgXact->vacuumFlags |= PROC_IN_VACUUM; + MyProc->vacuumFlagsCopy |= PROC_IN_VACUUM; if (params->is_wraparound) - MyPgXact->vacuumFlags |= PROC_VACUUM_FOR_WRAPAROUND; + MyProc->vacuumFlagsCopy |= PROC_VACUUM_FOR_WRAPAROUND; + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy; LWLockRelease(ProcArrayLock); } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index df1af9354ce..465f8893cd5 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2494,7 +2494,7 @@ do_autovacuum(void) tab->at_datname, tab->at_nspname, tab->at_relname); EmitErrorReport(); - /* this resets the PGXACT flags too */ + /* this resets ProcGlobal->vacuumFlags[i] too */ AbortOutOfAnyTransaction(); FlushErrorState(); MemoryContextResetAndDeleteChildren(PortalContext); @@ -2510,7 +2510,7 @@ do_autovacuum(void) did_vacuum = true; - /* the PGXACT flags are reset at the next end of transaction */ + /* ProcGlobal->vacuumFlags[i] are reset at the next end of xact */ /* be tidy */ deleted: @@ -2687,7 +2687,7 @@ perform_work_item(AutoVacuumWorkItem *workitem) cur_datname, cur_nspname, cur_relname); EmitErrorReport(); - /* this resets the PGXACT flags too */ + /* this resets ProcGlobal->vacuumFlags[i] too */ AbortOutOfAnyTransaction(); FlushErrorState(); MemoryContextResetAndDeleteChildren(PortalContext); diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 5adf253583b..d5a28821e14 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -163,7 +163,8 @@ StartupDecodingContext(List *output_plugin_options, if (!IsTransactionOrTransactionBlock()) { LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); - MyPgXact->vacuumFlags |= PROC_IN_LOGICAL_DECODING; + MyProc->vacuumFlagsCopy |= PROC_IN_LOGICAL_DECODING; + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy; LWLockRelease(ProcArrayLock); } diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 47851ec4c1a..0dcc6a90e79 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -468,7 +468,8 @@ ReplicationSlotRelease(void) /* might not have been set when we've been a plain slot */ LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); - MyPgXact->vacuumFlags &= ~PROC_IN_LOGICAL_DECODING; + MyProc->vacuumFlagsCopy &= ~PROC_IN_LOGICAL_DECODING; + ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy; LWLockRelease(ProcArrayLock); } diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 7a6efaafe26..384b5a8efb5 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -441,9 +441,12 @@ ProcArrayAdd(PGPROC *proc) (arrayP->numProcs - index) * sizeof(*arrayP->pgprocnos)); memmove(&ProcGlobal->xids[index + 1], &ProcGlobal->xids[index], (arrayP->numProcs - index) * sizeof(*ProcGlobal->xids)); + memmove(&ProcGlobal->vacuumFlags[index + 1], &ProcGlobal->vacuumFlags[index], + (arrayP->numProcs - index) * sizeof(*ProcGlobal->vacuumFlags)); arrayP->pgprocnos[index] = proc->pgprocno; ProcGlobal->xids[index] = proc->xidCopy; + ProcGlobal->vacuumFlags[index] = proc->vacuumFlagsCopy; arrayP->numProcs++; @@ -504,6 +507,7 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid) } Assert(TransactionIdIsValid(ProcGlobal->xids[proc->pgxactoff] == 0)); + ProcGlobal->vacuumFlags[proc->pgxactoff] = 0; for (index = 0; index < arrayP->numProcs; index++) { @@ -514,6 +518,8 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid) (arrayP->numProcs - index - 1) * sizeof(*arrayP->pgprocnos)); memmove(&ProcGlobal->xids[index], &ProcGlobal->xids[index + 1], (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->xids)); + memmove(&ProcGlobal->vacuumFlags[index], &ProcGlobal->vacuumFlags[index + 1], + (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->vacuumFlags)); arrayP->pgprocnos[arrayP->numProcs - 1] = -1; /* for debugging */ arrayP->numProcs--; @@ -592,14 +598,23 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) Assert(!TransactionIdIsValid(proc->xidCopy)); proc->lxid = InvalidLocalTransactionId; - /* must be cleared with xid/xmin: */ - pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; proc->xmin = InvalidTransactionId; proc->delayChkpt = false; /* be sure this is cleared in abort */ proc->recoveryConflictPending = false; Assert(pgxact->nxids == 0); Assert(pgxact->overflowed == false); + + /* must be cleared with xid/xmin: */ + /* avoid unnecessarily dirtying shared cachelines */ + if (proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK) + { + Assert(!LWLockHeldByMe(ProcArrayLock)); + LWLockAcquire(ProcArrayLock, LW_SHARED); + proc->vacuumFlagsCopy &= ~PROC_VACUUM_STATE_MASK; + ProcGlobal->vacuumFlags[proc->pgxactoff] = proc->vacuumFlagsCopy; + LWLockRelease(ProcArrayLock); + } } } @@ -620,12 +635,18 @@ ProcArrayEndTransactionInternal(PGPROC *proc, PGXACT *pgxact, ProcGlobal->xids[pgxactoff] = InvalidTransactionId; proc->xidCopy = InvalidTransactionId; proc->lxid = InvalidLocalTransactionId; - /* must be cleared with xid/xmin: */ - pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; proc->xmin = InvalidTransactionId; proc->delayChkpt = false; /* be sure this is cleared in abort */ proc->recoveryConflictPending = false; + /* must be cleared with xid/xmin: */ + /* avoid unnecessarily dirtying shared cachelines */ + if (proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK) + { + proc->vacuumFlagsCopy &= ~PROC_VACUUM_STATE_MASK; + ProcGlobal->vacuumFlags[proc->pgxactoff] = proc->vacuumFlagsCopy; + } + /* Clear the subtransaction-XID cache too while holding the lock */ pgxact->nxids = 0; pgxact->overflowed = false; @@ -785,9 +806,8 @@ ProcArrayClearTransaction(PGPROC *proc) proc->xmin = InvalidTransactionId; proc->recoveryConflictPending = false; - /* redundant, but just in case */ - pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - proc->delayChkpt = false; + Assert(!(proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK)); + Assert(!proc->delayChkpt); /* Clear the subtransaction-XID cache too */ pgxact->nxids = 0; @@ -1538,7 +1558,7 @@ ComputeTransactionHorizons(ComputedHorizons *h) { int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - PGXACT *pgxact = &allPgXact[pgprocno]; + int8 vacuumFlags = ProcGlobal->vacuumFlags[index]; TransactionId xid; TransactionId xmin; @@ -1555,10 +1575,6 @@ ComputeTransactionHorizons(ComputedHorizons *h) */ xmin = TransactionIdOlder(xmin, xid); - /* if neither is set, this proc doesn't influence the horizon */ - if (!TransactionIdIsValid(xmin)) - continue; - /* * Don't ignore any procs when determining which transactions might be * considered running. While slots should ensure logical decoding @@ -1573,7 +1589,7 @@ ComputeTransactionHorizons(ComputedHorizons *h) * removed, as long as pg_subtrans is not truncated) or doing logical * decoding (which manages xmin separately, check below). */ - if (pgxact->vacuumFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING)) + if (vacuumFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING)) continue; /* shared tables need to take backends in all database into account */ @@ -1908,6 +1924,7 @@ GetSnapshotData(Snapshot snapshot) size_t numProcs = arrayP->numProcs; TransactionId *xip = snapshot->xip; int workspace_count = 0; + uint8 *allVacuumFlags = ProcGlobal->vacuumFlags; /* * First collect set of pgxactoff/xids that need to be included in the @@ -1956,7 +1973,7 @@ GetSnapshotData(Snapshot snapshot) TransactionId xid = snapshot_workspace_xid[i]; int pgprocno = arrayP->pgprocnos[pgxactoff]; PGXACT *pgxact = &allPgXact[pgprocno]; - uint8 vacuumFlags = pgxact->vacuumFlags; + uint8 vacuumFlags = allVacuumFlags[pgxactoff]; int nsubxids; Assert(allProcs[arrayP->pgprocnos[pgxactoff]].pgxactoff == pgxactoff); @@ -2208,11 +2225,11 @@ ProcArrayInstallImportedXmin(TransactionId xmin, { int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - PGXACT *pgxact = &allPgXact[pgprocno]; + int vacuumFlags = ProcGlobal->vacuumFlags[index]; TransactionId xid; /* Ignore procs running LAZY VACUUM */ - if (pgxact->vacuumFlags & PROC_IN_VACUUM) + if (vacuumFlags & PROC_IN_VACUUM) continue; /* We are only interested in the specific virtual transaction. */ @@ -2901,12 +2918,12 @@ GetCurrentVirtualXIDs(TransactionId limitXmin, bool excludeXmin0, { int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - PGXACT *pgxact = &allPgXact[pgprocno]; + uint8 vacuumFlags = ProcGlobal->vacuumFlags[index]; if (proc == MyProc) continue; - if (excludeVacuum & pgxact->vacuumFlags) + if (excludeVacuum & vacuumFlags) continue; if (allDbs || proc->databaseId == MyDatabaseId) @@ -3321,7 +3338,7 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared) { int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - PGXACT *pgxact = &allPgXact[pgprocno]; + uint8 vacuumFlags = ProcGlobal->vacuumFlags[index]; if (proc->databaseId != databaseId) continue; @@ -3335,7 +3352,7 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared) else { (*nbackends)++; - if ((pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) && + if ((vacuumFlags & PROC_IS_AUTOVACUUM) && nautovacs < MAXAUTOVACPIDS) autovac_pids[nautovacs++] = proc->pid; } diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index beedc7947db..70e653bd3c9 100644 --- a/src/backend/storage/lmgr/deadlock.c +++ b/src/backend/storage/lmgr/deadlock.c @@ -544,7 +544,6 @@ FindLockCycleRecurseMember(PGPROC *checkProc, { PGPROC *proc; LOCK *lock = checkProc->waitLock; - PGXACT *pgxact; PROCLOCK *proclock; SHM_QUEUE *procLocks; LockMethod lockMethodTable; @@ -582,7 +581,6 @@ FindLockCycleRecurseMember(PGPROC *checkProc, PGPROC *leader; proc = proclock->tag.myProc; - pgxact = &ProcGlobal->allPgXact[proc->pgprocno]; leader = proc->lockGroupLeader == NULL ? proc : proc->lockGroupLeader; /* A proc never blocks itself or any other lock group member */ @@ -628,9 +626,11 @@ FindLockCycleRecurseMember(PGPROC *checkProc, * problems (which needs to read a different vacuumFlag * bit), but we don't do that here to avoid grabbing * ProcArrayLock. + * + * XXX: That's why this is using vacuumFlagsCopy. */ if (checkProc == MyProc && - pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) + proc->vacuumFlagsCopy & PROC_IS_AUTOVACUUM) blocking_autovacuum_proc = proc; /* We're done looking at this proclock */ diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 4dc588223c1..00f26d93c1a 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -113,6 +113,7 @@ ProcGlobalShmemSize(void) size = add_size(size, mul_size(NUM_AUXILIARY_PROCS, sizeof(PGXACT))); size = add_size(size, mul_size(max_prepared_xacts, sizeof(PGXACT))); size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->xids))); + size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->vacuumFlags))); return size; } @@ -230,6 +231,8 @@ InitProcGlobal(void) // XXX: Pad to cacheline (or even page?)! ProcGlobal->xids = (TransactionId *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->xids)); MemSet(ProcGlobal->xids, 0, TotalProcs * sizeof(*ProcGlobal->xids)); + ProcGlobal->vacuumFlags = (uint8 *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->vacuumFlags)); + MemSet(ProcGlobal->vacuumFlags, 0, TotalProcs * sizeof(*ProcGlobal->vacuumFlags)); for (i = 0; i < TotalProcs; i++) { @@ -412,10 +415,10 @@ InitProcess(void) MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; MyProc->delayChkpt = false; - MyPgXact->vacuumFlags = 0; + MyProc->vacuumFlagsCopy = 0; /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ if (IsAutoVacuumWorkerProcess()) - MyPgXact->vacuumFlags |= PROC_IS_AUTOVACUUM; + MyProc->vacuumFlagsCopy |= PROC_IS_AUTOVACUUM; MyProc->lwWaiting = false; MyProc->lwWaitMode = 0; MyProc->waitLock = NULL; @@ -594,7 +597,7 @@ InitAuxiliaryProcess(void) MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; MyProc->delayChkpt = false; - MyPgXact->vacuumFlags = 0; + MyProc->vacuumFlagsCopy = 0; MyProc->lwWaiting = false; MyProc->lwWaitMode = 0; MyProc->waitLock = NULL; @@ -1330,7 +1333,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM && allow_autovacuum_cancel) { PGPROC *autovac = GetBlockingAutoVacuumPgproc(); - PGXACT *autovac_pgxact = &ProcGlobal->allPgXact[autovac->pgprocno]; + uint8 vacuumFlags; LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); @@ -1338,8 +1341,9 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) * Only do it if the worker is not working to protect against Xid * wraparound. */ - if ((autovac_pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) && - !(autovac_pgxact->vacuumFlags & PROC_VACUUM_FOR_WRAPAROUND)) + vacuumFlags = ProcGlobal->vacuumFlags[proc->pgxactoff]; + if ((vacuumFlags & PROC_IS_AUTOVACUUM) && + !(vacuumFlags & PROC_VACUUM_FOR_WRAPAROUND)) { int pid = autovac->pid; StringInfoData locktagbuf; -- 2.25.0.114.g5b0ca878e0