From 75848bc74411130ede23995d0ab1aefb12c4c4b0 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 7 Apr 2020 17:36:23 -0700 Subject: [PATCH] snapshot scalability: Move delayChkpt from PGXACT to PGPROC. The goal of separating hotly accessed per-backend data from PGPROC into PGXACT is to make accesses fast (GetSnapshotData() in particular). But delayChkpt is not actually accessed frequently; only when starting a checkpoint. As it is frequently modified (multiple times in the course of a single transaction), storing it in the same cacheline as hotly accessed data unnecessarily dirties a contended cacheline. Therefore move delayChkpt to PGPROC. This is part of a larger series of patches intending to improve GetSnapshotData() scalability. It is committed and pushed separately, as it is independently beneficial (small but measurable win, limited by the other frequent modifications of PGXACT). Author: Andres Freund Reviewed-By: Robert Haas, Thomas Munro, David Rowley Discussion: https://postgr.es/m/20200301083601.ews6hz5dduc3w2se@alap3.anarazel.de --- src/backend/access/transam/multixact.c | 6 +++--- src/backend/access/transam/twophase.c | 10 +++++----- src/backend/access/transam/xact.c | 4 ++-- src/backend/access/transam/xloginsert.c | 2 +- src/backend/storage/buffer/bufmgr.c | 4 ++-- src/backend/storage/ipc/procarray.c | 14 ++++++-------- src/backend/storage/lmgr/proc.c | 4 ++-- src/include/storage/proc.h | 4 ++-- 8 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index fdd0394ffa..70d0e1c215 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -3058,8 +3058,8 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB) * crash/basebackup, even though the state of the data directory would * require it. */ - Assert(!MyPgXact->delayChkpt); - MyPgXact->delayChkpt = true; + Assert(!MyProc->delayChkpt); + MyProc->delayChkpt = true; /* WAL log truncation */ WriteMTruncateXlogRec(newOldestMultiDB, @@ -3085,7 +3085,7 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB) /* Then offsets */ PerformOffsetsTruncation(oldestMulti, newOldestMulti); - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; END_CRIT_SECTION(); LWLockRelease(MultiXactTruncationLock); diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 5adf956f41..2f7d4ed59a 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -465,7 +465,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, proc->lxid = (LocalTransactionId) xid; pgxact->xid = xid; pgxact->xmin = InvalidTransactionId; - pgxact->delayChkpt = false; + proc->delayChkpt = false; pgxact->vacuumFlags = 0; proc->pid = 0; proc->backendId = InvalidBackendId; @@ -1114,7 +1114,7 @@ EndPrepare(GlobalTransaction gxact) START_CRIT_SECTION(); - MyPgXact->delayChkpt = true; + MyProc->delayChkpt = true; XLogBeginInsert(); for (record = records.head; record != NULL; record = record->next) @@ -1157,7 +1157,7 @@ EndPrepare(GlobalTransaction gxact) * checkpoint starting after this will certainly see the gxact as a * candidate for fsyncing. */ - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; /* * Remember that we have this GlobalTransaction entry locked for us. If @@ -2204,7 +2204,7 @@ RecordTransactionCommitPrepared(TransactionId xid, START_CRIT_SECTION(); /* See notes in RecordTransactionCommit */ - MyPgXact->delayChkpt = true; + MyProc->delayChkpt = true; /* * Emit the XLOG commit record. Note that we mark 2PC commits as @@ -2252,7 +2252,7 @@ RecordTransactionCommitPrepared(TransactionId xid, TransactionIdCommitTree(xid, nchildren, children); /* Checkpoint can proceed now */ - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; END_CRIT_SECTION(); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 110ec228eb..6b1ae1f981 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1307,7 +1307,7 @@ RecordTransactionCommit(void) * a bit fuzzy, but it doesn't matter. */ START_CRIT_SECTION(); - MyPgXact->delayChkpt = true; + MyProc->delayChkpt = true; SetCurrentTransactionStopTimestamp(); @@ -1408,7 +1408,7 @@ RecordTransactionCommit(void) */ if (markXidCommitted) { - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; END_CRIT_SECTION(); } diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 5e032e7042..4259309dba 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -904,7 +904,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) /* * Ensure no checkpoint can change our view of RedoRecPtr. */ - Assert(MyPgXact->delayChkpt); + Assert(MyProc->delayChkpt); /* * Update RedoRecPtr so that we can make the right decision diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 7317ac8a2c..a7a39dd2a1 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3587,7 +3587,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) * essential that CreateCheckpoint waits for virtual transactions * rather than full transactionids. */ - MyPgXact->delayChkpt = delayChkpt = true; + MyProc->delayChkpt = delayChkpt = true; lsn = XLogSaveBufferForHint(buffer, buffer_std); } @@ -3620,7 +3620,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) UnlockBufHdr(bufHdr, buf_state); if (delayChkpt) - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; if (dirtied) { diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 281fe671bd..363000670b 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -436,7 +436,7 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) pgxact->xmin = InvalidTransactionId; /* must be cleared with xid/xmin: */ pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - pgxact->delayChkpt = false; /* be sure this is cleared in abort */ + proc->delayChkpt = false; /* be sure this is cleared in abort */ proc->recoveryConflictPending = false; Assert(pgxact->nxids == 0); @@ -458,7 +458,7 @@ ProcArrayEndTransactionInternal(PGPROC *proc, PGXACT *pgxact, pgxact->xmin = InvalidTransactionId; /* must be cleared with xid/xmin: */ pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - pgxact->delayChkpt = false; /* be sure this is cleared in abort */ + proc->delayChkpt = false; /* be sure this is cleared in abort */ proc->recoveryConflictPending = false; /* Clear the subtransaction-XID cache too while holding the lock */ @@ -616,7 +616,7 @@ ProcArrayClearTransaction(PGPROC *proc) /* redundant, but just in case */ pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - pgxact->delayChkpt = false; + proc->delayChkpt = false; /* Clear the subtransaction-XID cache too */ pgxact->nxids = 0; @@ -2257,7 +2257,7 @@ GetOldestSafeDecodingTransactionId(bool catalogOnly) * delaying checkpoint because they have critical actions in progress. * * Constructs an array of VXIDs of transactions that are currently in commit - * critical sections, as shown by having delayChkpt set in their PGXACT. + * critical sections, as shown by having delayChkpt set in their PGPROC. * * Returns a palloc'd array that should be freed by the caller. * *nvxids is the number of valid entries. @@ -2288,9 +2288,8 @@ GetVirtualXIDsDelayingChkpt(int *nvxids) { int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - PGXACT *pgxact = &allPgXact[pgprocno]; - if (pgxact->delayChkpt) + if (proc->delayChkpt) { VirtualTransactionId vxid; @@ -2328,12 +2327,11 @@ HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids) { int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - PGXACT *pgxact = &allPgXact[pgprocno]; VirtualTransactionId vxid; GET_VXID_FROM_PGPROC(vxid, *proc); - if (pgxact->delayChkpt && VirtualTransactionIdIsValid(vxid)) + if (proc->delayChkpt && VirtualTransactionIdIsValid(vxid)) { int i; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index baecb39787..04a5d595e4 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -397,7 +397,7 @@ InitProcess(void) MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; MyPgXact->vacuumFlags = 0; /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ if (IsAutoVacuumWorkerProcess()) @@ -579,7 +579,7 @@ InitAuxiliaryProcess(void) MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; - MyPgXact->delayChkpt = false; + MyProc->delayChkpt = false; MyPgXact->vacuumFlags = 0; MyProc->lwWaiting = false; MyProc->lwWaitMode = 0; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index d21780108b..ae4f573ab4 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -142,6 +142,8 @@ struct PGPROC LOCKMASK heldLocks; /* bitmask for lock types already held on this * lock object by this backend */ + bool delayChkpt; /* true if this proc delays checkpoint start */ + /* * Info to allow us to wait for synchronous replication, if needed. * waitLSN is InvalidXLogRecPtr if not waiting; set only by user backend. @@ -232,8 +234,6 @@ typedef struct PGXACT uint8 vacuumFlags; /* vacuum-related flags, see above */ bool overflowed; - bool delayChkpt; /* true if this proc delays checkpoint start; - * previously called InCommit */ uint8 nxids; } PGXACT;