agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v7 08/11] snapshot scalability: Move PGXACT->vacuumFlags to ProcGlobal->vacuumFlags.
Date: Mon, 6 Apr 2020 21:28:55 -0700
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
--filbd3fs2o265vea
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0009-snapshot-scalability-Move-subxact-info-from-PGXAC.patch"
view thread (6+ messages) latest in thread
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: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v7 08/11] snapshot scalability: Move PGXACT->vacuumFlags to ProcGlobal->vacuumFlags.
In-Reply-To: <no-message-id-195064@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox