From eb4a3be2735f19d700c9687b2ec28c3587b576c1 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 7 Apr 2022 10:24:26 -0400 Subject: [PATCH] Rename delayChkpt to delayChkptFlags. Before commit 412ad7a55639516f284cd0ef9757d6ae5c7abd43, delayChkpt was a Boolean. Now it's an integer. Extensions using it need to be appropriately updated, so let's rename the field to make sure that a hard compilation failure occurs. Replacing delayChkpt with delayChkptFlags made a few comments extend past 80 characters, so I reflowed them and changed some wording very slightly. Per suggestion from Tom Lane. --- src/backend/access/transam/multixact.c | 6 ++--- src/backend/access/transam/twophase.c | 28 ++++++++++++------------ src/backend/access/transam/xact.c | 14 ++++++------ src/backend/access/transam/xlog.c | 10 ++++----- src/backend/access/transam/xloginsert.c | 2 +- src/backend/catalog/storage.c | 6 ++--- src/backend/storage/buffer/bufmgr.c | 12 +++++----- src/backend/storage/ipc/procarray.c | 29 +++++++++++++------------ src/backend/storage/lmgr/proc.c | 4 ++-- src/include/storage/proc.h | 2 +- 10 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 9f65c600d0..45907d1b44 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -3088,8 +3088,8 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB) * crash/basebackup, even though the state of the data directory would * require it. */ - Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0); - MyProc->delayChkpt |= DELAY_CHKPT_START; + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0); + MyProc->delayChkptFlags |= DELAY_CHKPT_START; /* WAL log truncation */ WriteMTruncateXlogRec(newOldestMultiDB, @@ -3115,7 +3115,7 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB) /* Then offsets */ PerformOffsetsTruncation(oldestMulti, newOldestMulti); - MyProc->delayChkpt &= ~DELAY_CHKPT_START; + MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; END_CRIT_SECTION(); LWLockRelease(MultiXactTruncationLock); diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index b35da6f1aa..7632596008 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -479,7 +479,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, } proc->xid = xid; Assert(proc->xmin == InvalidTransactionId); - proc->delayChkpt = 0; + proc->delayChkptFlags = 0; proc->statusFlags = 0; proc->pid = 0; proc->databaseId = databaseid; @@ -1173,11 +1173,11 @@ EndPrepare(GlobalTransaction gxact) * Now writing 2PC state data to WAL. We let the WAL's CRC protection * cover us, so no need to calculate a separate CRC. * - * We have to set delayChkpt here, too; otherwise a checkpoint starting - * immediately after the WAL record is inserted could complete without - * fsync'ing our state file. (This is essentially the same kind of race - * condition as the COMMIT-to-clog-write case that RecordTransactionCommit - * uses delayChkpt for; see notes there.) + * We have to set DELAY_CHKPT_START here, too; otherwise a checkpoint + * starting immediately after the WAL record is inserted could complete + * without fsync'ing our state file. (This is essentially the same kind + * of race condition as the COMMIT-to-clog-write case that + * RecordTransactionCommit uses DELAY_CHKPT_START for; see notes there.) * * We save the PREPARE record's location in the gxact for later use by * CheckPointTwoPhase. @@ -1186,8 +1186,8 @@ EndPrepare(GlobalTransaction gxact) START_CRIT_SECTION(); - Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0); - MyProc->delayChkpt |= DELAY_CHKPT_START; + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0); + MyProc->delayChkptFlags |= DELAY_CHKPT_START; XLogBeginInsert(); for (record = records.head; record != NULL; record = record->next) @@ -1230,7 +1230,7 @@ EndPrepare(GlobalTransaction gxact) * checkpoint starting after this will certainly see the gxact as a * candidate for fsyncing. */ - MyProc->delayChkpt &= ~DELAY_CHKPT_START; + MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; /* * Remember that we have this GlobalTransaction entry locked for us. If @@ -1817,7 +1817,7 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon) * * Note that it isn't possible for there to be a GXACT with a * prepare_end_lsn set prior to the last checkpoint yet is marked invalid, - * because of the efforts with delayChkpt. + * because of the efforts with delayChkptFlags. */ LWLockAcquire(TwoPhaseStateLock, LW_SHARED); for (i = 0; i < TwoPhaseState->numPrepXacts; i++) @@ -2275,7 +2275,7 @@ ProcessTwoPhaseBuffer(TransactionId xid, * RecordTransactionCommitPrepared * * This is basically the same as RecordTransactionCommit (q.v. if you change - * this function): in particular, we must set the delayChkpt flag to avoid a + * this function): in particular, we must set DELAY_CHKPT_START to avoid a * race condition. * * We know the transaction made at least one XLOG entry (its PREPARE), @@ -2308,8 +2308,8 @@ RecordTransactionCommitPrepared(TransactionId xid, START_CRIT_SECTION(); /* See notes in RecordTransactionCommit */ - Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0); - MyProc->delayChkpt |= DELAY_CHKPT_START; + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0); + MyProc->delayChkptFlags |= DELAY_CHKPT_START; /* * Emit the XLOG commit record. Note that we mark 2PC commits as @@ -2358,7 +2358,7 @@ RecordTransactionCommitPrepared(TransactionId xid, TransactionIdCommitTree(xid, nchildren, children); /* Checkpoint can proceed now */ - MyProc->delayChkpt &= ~DELAY_CHKPT_START; + MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; END_CRIT_SECTION(); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index bf2fc08d94..53f3e7fd1a 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1387,14 +1387,14 @@ RecordTransactionCommit(void) * RecordTransactionAbort. That's because loss of a transaction abort * is noncritical; the presumption would be that it aborted, anyway. * - * It's safe to change the delayChkpt flag of our own backend without - * holding the ProcArrayLock, since we're the only one modifying it. - * This makes checkpoint's determination of which xacts are delayChkpt - * a bit fuzzy, but it doesn't matter. + * It's safe to change the delayChkptFlags flag of our own backend + * without holding the ProcArrayLock, since we're the only one + * modifying it. This makes checkpoint's determination of which xacts + * are delaying the checkpoint a bit fuzzy, but it doesn't matter. */ - Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0); + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0); START_CRIT_SECTION(); - MyProc->delayChkpt |= DELAY_CHKPT_START; + MyProc->delayChkptFlags |= DELAY_CHKPT_START; SetCurrentTransactionStopTimestamp(); @@ -1496,7 +1496,7 @@ RecordTransactionCommit(void) */ if (markXidCommitted) { - MyProc->delayChkpt &= ~DELAY_CHKPT_START; + MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; END_CRIT_SECTION(); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 6770c3ddba..a7814d4019 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6505,11 +6505,11 @@ CreateCheckPoint(int flags) * protected by different locks, but again that seems best on grounds of * minimizing lock contention.) * - * A transaction that has not yet set delayChkpt when we look cannot be at - * risk, since he's not inserted his commit record yet; and one that's - * already cleared it is not at risk either, since he's done fixing clog - * and we will correctly flush the update below. So we cannot miss any - * xacts we need to wait for. + * A transaction that has not yet set delayChkptFlags when we look cannot + * be at risk, since it has not inserted its commit record yet; and one + * that's already cleared it is not at risk either, since it's done fixing + * clog and we will correctly flush the update below. So we cannot miss + * any xacts we need to wait for. */ vxids = GetVirtualXIDsDelayingChkpt(&nvxids, DELAY_CHKPT_START); if (nvxids > 0) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 462e23503e..2ce9be2cc7 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -1011,7 +1011,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) /* * Ensure no checkpoint can change our view of RedoRecPtr. */ - Assert((MyProc->delayChkpt & DELAY_CHKPT_START) != 0); + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) != 0); /* * Update RedoRecPtr so that we can make the right decision diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index 9898701a43..e4d000d4fe 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -348,8 +348,8 @@ RelationTruncate(Relation rel, BlockNumber nblocks) * the blocks to not exist on disk at all, but not for them to have the * wrong contents. */ - Assert((MyProc->delayChkpt & DELAY_CHKPT_COMPLETE) == 0); - MyProc->delayChkpt |= DELAY_CHKPT_COMPLETE; + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_COMPLETE) == 0); + MyProc->delayChkptFlags |= DELAY_CHKPT_COMPLETE; /* * We WAL-log the truncation before actually truncating, which means @@ -397,7 +397,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks) smgrtruncate(RelationGetSmgr(rel), forks, nforks, blocks); /* We've done all the critical work, so checkpoints are OK now. */ - MyProc->delayChkpt &= ~DELAY_CHKPT_COMPLETE; + MyProc->delayChkptFlags &= ~DELAY_CHKPT_COMPLETE; /* * Update upper-level FSM pages to account for the truncation. This is diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 93c1ea2d9f..c12028ca0f 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4021,7 +4021,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) { XLogRecPtr lsn = InvalidXLogRecPtr; bool dirtied = false; - bool delayChkpt = false; + bool delayChkptFlags = false; uint32 buf_state; /* @@ -4071,9 +4071,9 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) * essential that CreateCheckPoint waits for virtual transactions * rather than full transactionids. */ - Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0); - MyProc->delayChkpt |= DELAY_CHKPT_START; - delayChkpt = true; + Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0); + MyProc->delayChkptFlags |= DELAY_CHKPT_START; + delayChkptFlags = true; lsn = XLogSaveBufferForHint(buffer, buffer_std); } @@ -4105,8 +4105,8 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) buf_state |= BM_DIRTY | BM_JUST_DIRTIED; UnlockBufHdr(bufHdr, buf_state); - if (delayChkpt) - MyProc->delayChkpt &= ~DELAY_CHKPT_START; + if (delayChkptFlags) + MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; if (dirtied) { diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 735763cc24..603f6aba71 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -700,7 +700,7 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) proc->xmin = InvalidTransactionId; /* be sure this is cleared in abort */ - proc->delayChkpt = 0; + proc->delayChkptFlags = 0; proc->recoveryConflictPending = false; @@ -742,7 +742,7 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid) proc->xmin = InvalidTransactionId; /* be sure this is cleared in abort */ - proc->delayChkpt = 0; + proc->delayChkptFlags = 0; proc->recoveryConflictPending = false; @@ -929,7 +929,7 @@ ProcArrayClearTransaction(PGPROC *proc) proc->recoveryConflictPending = false; Assert(!(proc->statusFlags & PROC_VACUUM_STATE_MASK)); - Assert(!proc->delayChkpt); + Assert(!proc->delayChkptFlags); /* * Need to increment completion count even though transaction hasn't @@ -3059,19 +3059,20 @@ 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 specified delayChkpt bits set in their - * PGPROC. + * critical sections, as shown by having specified delayChkptFlags bits set + * in their PGPROC. * * Returns a palloc'd array that should be freed by the caller. * *nvxids is the number of valid entries. * - * Note that because backends set or clear delayChkpt without holding any lock, - * the result is somewhat indeterminate, but we don't really care. Even in - * a multiprocessor with delayed writes to shared memory, it should be certain - * that setting of delayChkpt will propagate to shared memory when the backend - * takes a lock, so we cannot fail to see a virtual xact as delayChkpt if - * it's already inserted its commit record. Whether it takes a little while - * for clearing of delayChkpt to propagate is unimportant for correctness. + * Note that because backends set or clear delayChkptFlags without holding any + * lock, the result is somewhat indeterminate, but we don't really care. Even + * in a multiprocessor with delayed writes to shared memory, it should be + * certain that setting of delayChkptFlags will propagate to shared memory + * when the backend takes a lock, so we cannot fail to see a virtual xact as + * delayChkptFlags if it's already inserted its commit record. Whether it + * takes a little while for clearing of delayChkptFlags to propagate is + * unimportant for correctness. */ VirtualTransactionId * GetVirtualXIDsDelayingChkpt(int *nvxids, int type) @@ -3094,7 +3095,7 @@ GetVirtualXIDsDelayingChkpt(int *nvxids, int type) int pgprocno = arrayP->pgprocnos[index]; PGPROC *proc = &allProcs[pgprocno]; - if ((proc->delayChkpt & type) != 0) + if ((proc->delayChkptFlags & type) != 0) { VirtualTransactionId vxid; @@ -3138,7 +3139,7 @@ HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids, int type) GET_VXID_FROM_PGPROC(vxid, *proc); - if ((proc->delayChkpt & type) != 0 && + if ((proc->delayChkptFlags & type) != 0 && VirtualTransactionIdIsValid(vxid)) { int i; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index df080cd332..93d082c45e 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -393,7 +393,7 @@ InitProcess(void) MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; - MyProc->delayChkpt = 0; + MyProc->delayChkptFlags = 0; MyProc->statusFlags = 0; /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ if (IsAutoVacuumWorkerProcess()) @@ -578,7 +578,7 @@ InitAuxiliaryProcess(void) MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; - MyProc->delayChkpt = 0; + MyProc->delayChkptFlags = 0; MyProc->statusFlags = 0; MyProc->lwWaiting = false; MyProc->lwWaitMode = 0; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 36ecf7d005..21bef4c6dc 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -226,7 +226,7 @@ struct PGPROC pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition * started */ - int delayChkpt; /* for DELAY_CHKPT_* flags */ + int delayChkptFlags; /* for DELAY_CHKPT_* flags */ uint8 statusFlags; /* this backend's status flags, see PROC_* * above. mirrored in -- 2.24.3 (Apple Git-128)