At Wed, 06 Apr 2022 15:53:32 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
> At Wed, 06 Apr 2022 15:31:53 +0900 (JST), Kyotaro Horiguchi
> <horikyota....@gmail.com> wrote in
> > At Wed, 06 Apr 2022 14:30:37 +0900 (JST), Kyotaro Horiguchi
> > <horikyota....@gmail.com> wrote in
> > > So if we don't want to move any member in PGPROC, we do:
> > >
> > > 14: after statusFlags.
> > > 13: after delayChkpt.
> > > 12-10: after syncRepState (and before syncRepLinks).
> > >
> > > If we allow to shift some members, the new flag can be placed more
> > > saner place.
> > >
> > > 14: after delayChkpt ((uint8)statusFlags moves forward by 1 byte)
> > > 13: after delayChkpt (no member moves)
> > > 12-10: after subxids ((bool)procArrayGroupMember moves forward by 1 byte)
> > >
> > > I continue working on the last direction above.
> >
> > Hmm. That is ABI break. I go with the first way.
>
> By the way, the patch for -14 changed the sigunature of two public
> functions.
>
> -GetVirtualXIDsDelayingChkpt(int *nvxids)
> +GetVirtualXIDsDelayingChkpt(int *nvxids, int type)
>
> -HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids)
> +HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids, int
> type)
>
> Do I need to restore the signature?
For master, renamed delayChkpt to delayChkptFlags and changed it to uint8.
For 14, restored delayChkpt to bool and added delayChkptEnd into a gap in
PGPROC, then restored the signature of the two functions above to before the
patch. Then added a new functions ..DelayingChkptEnd().
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 307c295369d24e8a0ec4d3977ce1547162e7eeb9 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Wed, 6 Apr 2022 14:47:02 +0900
Subject: [PATCH v1] Rename and change type of delayChkpt
Commit 412ad7a556 changed the type of PGPROC.delayChkpt, but did not
change its name. Rename it to delayChkptFlags so as to ensure that
any code touching it has to be inspected and updated.
On the way renaming the struct member, narrow the width of the member
to uint8. It has been inadvertently widen from bool to int.
Note: Comments are not intentionally reflowed so that this can be
easily reviewed.
---
src/backend/access/transam/multixact.c | 6 +++---
src/backend/access/transam/twophase.c | 22 +++++++++++-----------
src/backend/access/transam/xact.c | 12 ++++++------
src/backend/access/transam/xlog.c | 2 +-
src/backend/access/transam/xloginsert.c | 2 +-
src/backend/catalog/storage.c | 6 +++---
src/backend/storage/buffer/bufmgr.c | 6 +++---
src/backend/storage/ipc/procarray.c | 22 +++++++++++-----------
src/backend/storage/lmgr/proc.c | 4 ++--
src/include/storage/proc.h | 6 +++---
10 files changed, 44 insertions(+), 44 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 4dc8ccc12b..0a0e35e79f 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -475,7 +475,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;
@@ -1151,11 +1151,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
+ * We have to set delayChkptFlags 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.)
+ * uses delayChkptFlags for; see notes there.)
*
* We save the PREPARE record's location in the gxact for later use by
* CheckPointTwoPhase.
@@ -1164,8 +1164,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)
@@ -1208,7 +1208,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
@@ -1780,7 +1780,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++)
@@ -2236,7 +2236,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 the delayChkptFlags to avoid a
* race condition.
*
* We know the transaction made at least one XLOG entry (its PREPARE),
@@ -2267,8 +2267,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
@@ -2316,7 +2316,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 3596a7d734..c018248f80 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1382,14 +1382,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
+ * It's safe to change the delayChkptFlags 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.
+ * This makes checkpoint's determination of which xacts are
+ * delayChkptFlags 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();
@@ -1490,7 +1490,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 17a56152f1..61cd5f8773 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6511,7 +6511,7 @@ 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
+ * A transaction that has not yet set delayChkptFlags 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
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 d73a40c1bc..f5cbb12e9c 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4067,8 +4067,8 @@ 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;
+ Assert((MyProc->delayChkptFlags & DELAY_CHKPT_START) == 0);
+ MyProc->delayChkptFlags |= DELAY_CHKPT_START;
delayChkpt = true;
lsn = XLogSaveBufferForHint(buffer, buffer_std);
}
@@ -4102,7 +4102,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
UnlockBufHdr(bufHdr, buf_state);
if (delayChkpt)
- MyProc->delayChkpt &= ~DELAY_CHKPT_START;
+ 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..cc9a9ae771 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,19 @@ 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,
+ * 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 delayChkpt will propagate to shared memory when the backend
- * takes a lock, so we cannot fail to see a virtual xact as delayChkpt if
+ * 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 delayChkpt to propagate is unimportant for correctness.
+ * for clearing of delayChkptFlags to propagate is unimportant for correctness.
*/
VirtualTransactionId *
GetVirtualXIDsDelayingChkpt(int *nvxids, int type)
@@ -3094,7 +3094,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 +3138,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..ddf859e1ea 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -87,7 +87,7 @@ struct XidCache
#define INVALID_PGPROCNO PG_INT32_MAX
/*
- * Flags for PGPROC.delayChkpt
+ * Flags for PGPROC.delayChkptFlags
*
* These flags can be used to delay the start or completion of a checkpoint
* for short periods. A flag is in effect if the corresponding bit is set in
@@ -145,7 +145,7 @@ typedef enum
* but its myProcLocks[] lists are valid.
*
* We allow many fields of this struct to be accessed without locks, such as
- * delayChkpt and isBackgroundWorker. However, keep in mind that writing
+ * delayChkptFlags and isBackgroundWorker. However, keep in mind that writing
* mirrored ones (see below) requires holding ProcArrayLock or XidGenLock in
* at least shared mode, so that pgxactoff does not change concurrently.
*
@@ -226,7 +226,7 @@ struct PGPROC
pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition
* started */
- int delayChkpt; /* for DELAY_CHKPT_* flags */
+ uint8 delayChkptFlags;/* for DELAY_CHKPT_* flags */
uint8 statusFlags; /* this backend's status flags, see PROC_*
* above. mirrored in
--
2.27.0
>From 3cf7523f3977d0b1a56844e884f53a10501e38e5 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Wed, 6 Apr 2022 15:47:50 +0900
Subject: [PATCH v1] Fix ABI/API break
Commit bbace5697d caused ABI break by chainging the type and width of
PGPROC.delayChkpt and API break by changing the signature of some
public functions. Restore the member and function signature to
previous state and add delayChkptEnd to an existing gap in PGPROC
struct.
---
src/backend/access/transam/multixact.c | 6 +--
src/backend/access/transam/twophase.c | 13 +++---
src/backend/access/transam/xact.c | 5 +--
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 | 6 +--
src/backend/storage/ipc/procarray.c | 59 +++++++++++++++++++------
src/backend/storage/lmgr/proc.c | 6 ++-
src/include/storage/proc.h | 42 +++---------------
src/include/storage/procarray.h | 7 ++-
11 files changed, 81 insertions(+), 81 deletions(-)
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 50d8bab9e2..b643564f16 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -3075,8 +3075,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->delayChkpt);
+ MyProc->delayChkpt = true;
/* WAL log truncation */
WriteMTruncateXlogRec(newOldestMultiDB,
@@ -3102,7 +3102,7 @@ TruncateMultiXact(MultiXactId newOldestMulti, Oid newOldestMultiDB)
/* Then offsets */
PerformOffsetsTruncation(oldestMulti, newOldestMulti);
- MyProc->delayChkpt &= ~DELAY_CHKPT_START;
+ 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 dea3f485f7..911d93fbf4 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -474,7 +474,8 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
}
proc->xid = xid;
Assert(proc->xmin == InvalidTransactionId);
- proc->delayChkpt = 0;
+ proc->delayChkpt = false;
+ proc->delayChkptEnd = false;
proc->statusFlags = 0;
proc->pid = 0;
proc->databaseId = databaseid;
@@ -1165,8 +1166,7 @@ EndPrepare(GlobalTransaction gxact)
START_CRIT_SECTION();
- Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0);
- MyProc->delayChkpt |= DELAY_CHKPT_START;
+ MyProc->delayChkpt = true;
XLogBeginInsert();
for (record = records.head; record != NULL; record = record->next)
@@ -1209,7 +1209,7 @@ EndPrepare(GlobalTransaction gxact)
* checkpoint starting after this will certainly see the gxact as a
* candidate for fsyncing.
*/
- MyProc->delayChkpt &= ~DELAY_CHKPT_START;
+ MyProc->delayChkpt = false;
/*
* Remember that we have this GlobalTransaction entry locked for us. If
@@ -2276,8 +2276,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
START_CRIT_SECTION();
/* See notes in RecordTransactionCommit */
- Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0);
- MyProc->delayChkpt |= DELAY_CHKPT_START;
+ MyProc->delayChkpt = true;
/*
* Emit the XLOG commit record. Note that we mark 2PC commits as
@@ -2325,7 +2324,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
TransactionIdCommitTree(xid, nchildren, children);
/* Checkpoint can proceed now */
- MyProc->delayChkpt &= ~DELAY_CHKPT_START;
+ MyProc->delayChkpt = false;
END_CRIT_SECTION();
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index c5e7261921..514044f3db 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1335,9 +1335,8 @@ RecordTransactionCommit(void)
* This makes checkpoint's determination of which xacts are delayChkpt
* a bit fuzzy, but it doesn't matter.
*/
- Assert((MyProc->delayChkpt & DELAY_CHKPT_START) == 0);
START_CRIT_SECTION();
- MyProc->delayChkpt |= DELAY_CHKPT_START;
+ MyProc->delayChkpt = true;
SetCurrentTransactionStopTimestamp();
@@ -1438,7 +1437,7 @@ RecordTransactionCommit(void)
*/
if (markXidCommitted)
{
- MyProc->delayChkpt &= ~DELAY_CHKPT_START;
+ MyProc->delayChkpt = false;
END_CRIT_SECTION();
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index d889d69387..8edf0fa646 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9228,27 +9228,25 @@ CreateCheckPoint(int flags)
* 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);
+ vxids = GetVirtualXIDsDelayingChkpt(&nvxids);
if (nvxids > 0)
{
do
{
pg_usleep(10000L); /* wait for 10 msec */
- } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids,
- DELAY_CHKPT_START));
+ } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids));
}
pfree(vxids);
CheckPointGuts(checkPoint.redo, flags);
- vxids = GetVirtualXIDsDelayingChkpt(&nvxids, DELAY_CHKPT_COMPLETE);
+ vxids = GetVirtualXIDsDelayingChkptEnd(&nvxids);
if (nvxids > 0)
{
do
{
pg_usleep(10000L); /* wait for 10 msec */
- } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids,
- DELAY_CHKPT_COMPLETE));
+ } while (HaveVirtualXIDsDelayingChkptEnd(vxids, nvxids));
}
pfree(vxids);
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index 1af4a90c41..b153fad594 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -925,7 +925,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->delayChkpt);
/*
* 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 fa5682dce8..b7fc491a68 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -338,8 +338,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->delayChkptEnd);
+ MyProc->delayChkptEnd = true;
/*
* We WAL-log the truncation before actually truncating, which means
@@ -387,7 +387,7 @@ RelationTruncate(Relation rel, BlockNumber nblocks)
smgrtruncate(rel->rd_smgr, forks, nforks, blocks);
/* We've done all the critical work, so checkpoints are OK now. */
- MyProc->delayChkpt &= ~DELAY_CHKPT_COMPLETE;
+ MyProc->delayChkptEnd = false;
/*
* 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 a55545a187..ffc6056c60 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3946,9 +3946,7 @@ 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;
+ MyProc->delayChkpt = delayChkpt = true;
lsn = XLogSaveBufferForHint(buffer, buffer_std);
}
@@ -3981,7 +3979,7 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
UnlockBufHdr(bufHdr, buf_state);
if (delayChkpt)
- MyProc->delayChkpt &= ~DELAY_CHKPT_START;
+ MyProc->delayChkpt = false;
if (dirtied)
{
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index ae71d7538b..024d26c15e 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -356,6 +356,11 @@ static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
TransactionId xid);
static void GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons);
+static VirtualTransactionId *GetVirtualXIDsDelayingChkptGuts(int *nvxids,
+ bool start);
+static bool HaveVirtualXIDsDelayingChkptGuts(VirtualTransactionId *vxids,
+ int nvxids, bool start);
+
/*
* Report shared-memory space needed by CreateSharedProcArray.
*/
@@ -690,8 +695,9 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
proc->lxid = InvalidLocalTransactionId;
proc->xmin = InvalidTransactionId;
- /* be sure this is cleared in abort */
- proc->delayChkpt = 0;
+ /* be sure these are cleared in abort */
+ proc->delayChkpt = false;
+ proc->delayChkptEnd = false;
proc->recoveryConflictPending = false;
@@ -732,8 +738,9 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
proc->lxid = InvalidLocalTransactionId;
proc->xmin = InvalidTransactionId;
- /* be sure this is cleared in abort */
- proc->delayChkpt = 0;
+ /* be sure these are cleared in abort */
+ proc->delayChkpt = false;
+ proc->delayChkptEnd = false;
proc->recoveryConflictPending = false;
@@ -921,6 +928,7 @@ ProcArrayClearTransaction(PGPROC *proc)
Assert(!(proc->statusFlags & PROC_VACUUM_STATE_MASK));
Assert(!proc->delayChkpt);
+ Assert(!proc->delayChkptEnd);
/*
* Need to increment completion count even though transaction hasn't
@@ -3049,8 +3057,9 @@ 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 the appropriate PGPROC flag set in
+ * their PGPROC. delayChkpt is checked if start is true and delayChkptEnd
+ * otherwise.
*
* Returns a palloc'd array that should be freed by the caller.
* *nvxids is the number of valid entries.
@@ -3064,15 +3073,26 @@ GetOldestSafeDecodingTransactionId(bool catalogOnly)
* for clearing of delayChkpt to propagate is unimportant for correctness.
*/
VirtualTransactionId *
-GetVirtualXIDsDelayingChkpt(int *nvxids, int type)
+GetVirtualXIDsDelayingChkpt(int *nvxids)
+{
+ return GetVirtualXIDsDelayingChkptGuts(nvxids, true);
+}
+
+VirtualTransactionId *
+GetVirtualXIDsDelayingChkptEnd(int *nvxids)
+{
+ return GetVirtualXIDsDelayingChkptGuts(nvxids, false);
+}
+
+
+static VirtualTransactionId *
+GetVirtualXIDsDelayingChkptGuts(int *nvxids, bool start)
{
VirtualTransactionId *vxids;
ProcArrayStruct *arrayP = procArray;
int count = 0;
int index;
- Assert(type != 0);
-
/* allocate what's certainly enough result space */
vxids = (VirtualTransactionId *)
palloc(sizeof(VirtualTransactionId) * arrayP->maxProcs);
@@ -3084,7 +3104,7 @@ GetVirtualXIDsDelayingChkpt(int *nvxids, int type)
int pgprocno = arrayP->pgprocnos[index];
PGPROC *proc = &allProcs[pgprocno];
- if ((proc->delayChkpt & type) != 0)
+ if (start ? proc->delayChkpt : proc->delayChkptEnd)
{
VirtualTransactionId vxid;
@@ -3110,14 +3130,25 @@ GetVirtualXIDsDelayingChkpt(int *nvxids, int type)
* those numbers should be small enough for it not to be a problem.
*/
bool
-HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids, int type)
+HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids)
+{
+ return HaveVirtualXIDsDelayingChkptGuts(vxids, nvxids, false);
+}
+
+bool
+HaveVirtualXIDsDelayingChkptEnd(VirtualTransactionId *vxids, int nvxids)
+{
+ return HaveVirtualXIDsDelayingChkptGuts(vxids, nvxids, true);
+}
+
+static bool
+HaveVirtualXIDsDelayingChkptGuts(VirtualTransactionId *vxids, int nvxids,
+ bool start)
{
bool result = false;
ProcArrayStruct *arrayP = procArray;
int index;
- Assert(type != 0);
-
LWLockAcquire(ProcArrayLock, LW_SHARED);
for (index = 0; index < arrayP->numProcs; index++)
@@ -3128,7 +3159,7 @@ HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids, int type)
GET_VXID_FROM_PGPROC(vxid, *proc);
- if ((proc->delayChkpt & type) != 0 &&
+ if ((start ? proc->delayChkpt : proc->delayChkptEnd) &&
VirtualTransactionIdIsValid(vxid))
{
int i;
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index c50a419a54..4d2db6dff2 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -394,7 +394,8 @@ InitProcess(void)
MyProc->roleId = InvalidOid;
MyProc->tempNamespaceId = InvalidOid;
MyProc->isBackgroundWorker = IsBackgroundWorker;
- MyProc->delayChkpt = 0;
+ MyProc->delayChkpt = false;
+ MyProc->delayChkptEnd = false;
MyProc->statusFlags = 0;
/* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */
if (IsAutoVacuumWorkerProcess())
@@ -579,7 +580,8 @@ InitAuxiliaryProcess(void)
MyProc->roleId = InvalidOid;
MyProc->tempNamespaceId = InvalidOid;
MyProc->isBackgroundWorker = IsBackgroundWorker;
- MyProc->delayChkpt = 0;
+ MyProc->delayChkpt = false;
+ MyProc->delayChkptEnd = false;
MyProc->statusFlags = 0;
MyProc->lwWaiting = false;
MyProc->lwWaitMode = 0;
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index b78012ec2b..5000818ca4 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -86,41 +86,6 @@ struct XidCache
*/
#define INVALID_PGPROCNO PG_INT32_MAX
-/*
- * Flags for PGPROC.delayChkpt
- *
- * These flags can be used to delay the start or completion of a checkpoint
- * for short periods. A flag is in effect if the corresponding bit is set in
- * the PGPROC of any backend.
- *
- * For our purposes here, a checkpoint has three phases: (1) determine the
- * location to which the redo pointer will be moved, (2) write all the
- * data durably to disk, and (3) WAL-log the checkpoint.
- *
- * Setting DELAY_CHKPT_START prevents the system from moving from phase 1
- * to phase 2. This is useful when we are performing a WAL-logged modification
- * of data that will be flushed to disk in phase 2. By setting this flag
- * before writing WAL and clearing it after we've both written WAL and
- * performed the corresponding modification, we ensure that if the WAL record
- * is inserted prior to the new redo point, the corresponding data changes will
- * also be flushed to disk before the checkpoint can complete. (In the
- * extremely common case where the data being modified is in shared buffers
- * and we acquire an exclusive content lock on the relevant buffers before
- * writing WAL, this mechanism is not needed, because phase 2 will block
- * until we release the content lock and then flush the modified data to
- * disk.)
- *
- * Setting DELAY_CHKPT_COMPLETE prevents the system from moving from phase 2
- * to phase 3. This is useful if we are performing a WAL-logged operation that
- * might invalidate buffers, such as relation truncation. In this case, we need
- * to ensure that any buffers which were invalidated and thus not flushed by
- * the checkpoint are actaully destroyed on disk. Replay can cope with a file
- * or block that doesn't exist, but not with a block that has the wrong
- * contents.
- */
-#define DELAY_CHKPT_START (1<<0)
-#define DELAY_CHKPT_COMPLETE (1<<1)
-
typedef enum
{
PROC_WAIT_STATUS_OK,
@@ -226,11 +191,16 @@ struct PGPROC
pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition
* started */
- int delayChkpt; /* for DELAY_CHKPT_* flags */
+ bool delayChkpt; /* true if this proc delays checkpoint start */
uint8 statusFlags; /* this backend's status flags, see PROC_*
* above. mirrored in
* ProcGlobal->statusFlags[pgxactoff] */
+ /*
+ * delayChkptEnd is placed here to prevent ABI breakage by moving other
+ * struct members around.
+ */
+ bool delayChkptEnd; /* true if this proc delays checkpoint end */
/*
* Info to allow us to wait for synchronous replication, if needed.
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 93de230a32..2cc9663f8f 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -59,9 +59,12 @@ extern TransactionId GetOldestActiveTransactionId(void);
extern TransactionId GetOldestSafeDecodingTransactionId(bool catalogOnly);
extern void GetReplicationHorizons(TransactionId *slot_xmin, TransactionId *catalog_xmin);
-extern VirtualTransactionId *GetVirtualXIDsDelayingChkpt(int *nvxids, int type);
+extern VirtualTransactionId *GetVirtualXIDsDelayingChkpt(int *nvxids);
extern bool HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids,
- int nvxids, int type);
+ int nvxids);
+extern VirtualTransactionId *GetVirtualXIDsDelayingChkptEnd(int *nvxids);
+extern bool HaveVirtualXIDsDelayingChkptEnd(VirtualTransactionId *vxids,
+ int nvxids);
extern PGPROC *BackendPidGetProc(int pid);
extern PGPROC *BackendPidGetProcWithLock(int pid);
--
2.27.0
