diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 5139af69bbf4dc1f59bed6a29c17cfc8aec29e59..3a0b190abcfe520c72c4efc8f95ab2b7d4f604e4 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -318,7 +318,7 @@ MarkAsPreparing(TransactionId xid, const char *gid, proc->lxid = (LocalTransactionId) xid; pgxact->xid = xid; pgxact->xmin = InvalidTransactionId; - pgxact->inCommit = false; + pgxact->delayChkpt = false; pgxact->vacuumFlags = 0; proc->pid = 0; proc->backendId = InvalidBackendId; @@ -1034,18 +1034,18 @@ EndPrepare(GlobalTransaction gxact) * odds of a PANIC actually occurring should be very tiny given that we * were able to write the bogus CRC above. * - * We have to set inCommit here, too; otherwise a checkpoint starting + * 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 inCommit for; see notes there.) + * uses delayChkpt for; see notes there.) * * We save the PREPARE record's location in the gxact for later use by * CheckPointTwoPhase. */ START_CRIT_SECTION(); - MyPgXact->inCommit = true; + MyPgXact->delayChkpt = true; gxact->prepare_lsn = XLogInsert(RM_XACT_ID, XLOG_XACT_PREPARE, records.head); @@ -1086,7 +1086,7 @@ EndPrepare(GlobalTransaction gxact) * checkpoint starting after this will certainly see the gxact as a * candidate for fsyncing. */ - MyPgXact->inCommit = false; + MyPgXact->delayChkpt = false; END_CRIT_SECTION(); @@ -1972,7 +1972,7 @@ RecoverPreparedTransactions(void) * RecordTransactionCommitPrepared * * This is basically the same as RecordTransactionCommit: in particular, - * we must set the inCommit flag to avoid a race condition. + * we must set the delayChkpt flag to avoid a race condition. * * We know the transaction made at least one XLOG entry (its PREPARE), * so it is never possible to optimize out the commit record. @@ -1995,7 +1995,7 @@ RecordTransactionCommitPrepared(TransactionId xid, START_CRIT_SECTION(); /* See notes in RecordTransactionCommit */ - MyPgXact->inCommit = true; + MyPgXact->delayChkpt = true; /* Emit the XLOG commit record */ xlrec.xid = xid; @@ -2053,7 +2053,7 @@ RecordTransactionCommitPrepared(TransactionId xid, TransactionIdCommitTree(xid, nchildren, children); /* Checkpoint can proceed now */ - MyPgXact->inCommit = false; + MyPgXact->delayChkpt = false; END_CRIT_SECTION(); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 349bdbcd9298225a9f6d7b539681ce0160e11842..a36c8061a2bb2f7846798c7b3ce5dc237919582d 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1001,13 +1001,13 @@ 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 inCommit flag of our own backend without + * 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 inCommit a + * This makes checkpoint's determination of which xacts are delayChkpt a * bit fuzzy, but it doesn't matter. */ START_CRIT_SECTION(); - MyPgXact->inCommit = true; + MyPgXact->delayChkpt = true; SetCurrentTransactionStopTimestamp(); @@ -1160,7 +1160,7 @@ RecordTransactionCommit(void) */ if (markXidCommitted) { - MyPgXact->inCommit = false; + MyPgXact->delayChkpt = false; END_CRIT_SECTION(); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 411807006ac6cd49ac47774d8d7f6171254293cf..2b579f9b727f26f5d08a792d3d76dfa70e13972f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6884,8 +6884,8 @@ CreateCheckPoint(int flags) XLogRecData rdata; uint32 freespace; XLogSegNo _logSegNo; - TransactionId *inCommitXids; - int nInCommit; + VirtualTransactionId *vxids; + int nvxids; /* * An end-of-recovery checkpoint is really a shutdown checkpoint, just @@ -7056,9 +7056,14 @@ CreateCheckPoint(int flags) TRACE_POSTGRESQL_CHECKPOINT_START(flags); /* - * Before flushing data, we must wait for any transactions that are - * currently in their commit critical sections. If an xact inserted its - * commit record into XLOG just before the REDO point, then a crash + * In some cases there are groups of actions that must all occur on + * one side or the other of a checkpoint record. Before flushing the + * checkpoint record we must explicitly wait for any backend currently + * performing those groups of actions. + * + * One example is end of transaction, so we must wait for any transactions + * that are currently in commit critical sections. If an xact inserted + * its commit record into XLOG just before the REDO point, then a crash * restart from the REDO point would not replay that record, which means * that our flushing had better include the xact's update of pg_clog. So * we wait till he's out of his commit critical section before proceeding. @@ -7073,21 +7078,24 @@ 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 inCommit when we look cannot be at + * 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. */ - nInCommit = GetTransactionsInCommit(&inCommitXids); - if (nInCommit > 0) + vxids = GetVirtualXIDsDelayingChkpt(&nvxids); + if (nvxids > 0) { + uint nwaits = 0; + do { pg_usleep(10000L); /* wait for 10 msec */ - } while (HaveTransactionsInCommit(inCommitXids, nInCommit)); + nwaits++; + } while (HaveVirtualXIDsDelayingChkpt(vxids, nvxids)); } - pfree(inCommitXids); + pfree(vxids); /* * Get the other info we need for the checkpoint record. diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index a98358daf53c0bd398944b4ac205f96e8b807c39..94f58a9b9d6d1ea8055b9b154cb0fba7732edad9 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -400,7 +400,7 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) pgxact->xmin = InvalidTransactionId; /* must be cleared with xid/xmin: */ pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - pgxact->inCommit = false; /* be sure this is cleared in abort */ + pgxact->delayChkpt = false; /* be sure this is cleared in abort */ proc->recoveryConflictPending = false; /* Clear the subtransaction-XID cache too while holding the lock */ @@ -427,7 +427,7 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid) pgxact->xmin = InvalidTransactionId; /* must be cleared with xid/xmin: */ pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - pgxact->inCommit = false; /* be sure this is cleared in abort */ + pgxact->delayChkpt = false; /* be sure this is cleared in abort */ proc->recoveryConflictPending = false; Assert(pgxact->nxids == 0); @@ -462,7 +462,7 @@ ProcArrayClearTransaction(PGPROC *proc) /* redundant, but just in case */ pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK; - pgxact->inCommit = false; + pgxact->delayChkpt = false; /* Clear the subtransaction-XID cache too */ pgxact->nxids = 0; @@ -1778,65 +1778,70 @@ GetOldestActiveTransactionId(void) } /* - * GetTransactionsInCommit -- Get the XIDs of transactions that are committing + * GetVirtualXIDsDelayingChkpt -- Get the VXIDs of transactions that are + * delaying checkpoint because they have critical actions in progress. * - * Constructs an array of XIDs of transactions that are currently in commit - * critical sections, as shown by having inCommit set in their PGXACT entries. + * Constructs an array of VXIDs of transactions that are currently in commit + * critical sections, as shown by having delayChkpt set in their PGXACT. * - * *xids_p is set to a palloc'd array that should be freed by the caller. - * The return value is the number of valid entries. + * 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 inCommit without holding any lock, + * 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 inCommit will propagate to shared memory when the backend - * takes the WALInsertLock, so we cannot fail to see an xact as inCommit if + * that setting of delayChkpt will propagate to shared memory when the backend + * takes a lock, so we cannot fail to see an virtual xact as delayChkpt if * it's already inserted its commit record. Whether it takes a little while - * for clearing of inCommit to propagate is unimportant for correctness. + * for clearing of delayChkpt to propagate is unimportant for correctness. */ -int -GetTransactionsInCommit(TransactionId **xids_p) +VirtualTransactionId * +GetVirtualXIDsDelayingChkpt(int *nvxids) { + VirtualTransactionId *vxids; ProcArrayStruct *arrayP = procArray; - TransactionId *xids; - int nxids; + int count = 0; int index; - xids = (TransactionId *) palloc(arrayP->maxProcs * sizeof(TransactionId)); - nxids = 0; + /* allocate what's certainly enough result space */ + vxids = (VirtualTransactionId *) + palloc(sizeof(VirtualTransactionId) * arrayP->maxProcs); LWLockAcquire(ProcArrayLock, LW_SHARED); for (index = 0; index < arrayP->numProcs; index++) { - int pgprocno = arrayP->pgprocnos[index]; - volatile PGXACT *pgxact = &allPgXact[pgprocno]; - TransactionId pxid; + int pgprocno = arrayP->pgprocnos[index]; + volatile PGPROC *proc = &allProcs[pgprocno]; + volatile PGXACT *pgxact = &allPgXact[pgprocno]; - /* Fetch xid just once - see GetNewTransactionId */ - pxid = pgxact->xid; + if (pgxact->delayChkpt) + { + VirtualTransactionId vxid; - if (pgxact->inCommit && TransactionIdIsValid(pxid)) - xids[nxids++] = pxid; + GET_VXID_FROM_PGPROC(vxid, *proc); + if (VirtualTransactionIdIsValid(vxid)) + vxids[count++] = vxid; + } } LWLockRelease(ProcArrayLock); - *xids_p = xids; - return nxids; + *nvxids = count; + return vxids; } /* - * HaveTransactionsInCommit -- Are any of the specified XIDs in commit? + * HaveVirtualXIDsDelayingChkpt -- Are any of the specified VXIDs delaying? * - * This is used with the results of GetTransactionsInCommit to see if any - * of the specified XIDs are still in their commit critical sections. + * This is used with the results of GetVirtualXIDsDelayingChkpt to see if any + * of the specified VXIDs are still in critical sections of code. * - * Note: this is O(N^2) in the number of xacts that are/were in commit, but + * Note: this is O(N^2) in the number of vxacts that are/were delaying, but * those numbers should be small enough for it not to be a problem. */ bool -HaveTransactionsInCommit(TransactionId *xids, int nxids) +HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids) { bool result = false; ProcArrayStruct *arrayP = procArray; @@ -1844,30 +1849,32 @@ HaveTransactionsInCommit(TransactionId *xids, int nxids) LWLockAcquire(ProcArrayLock, LW_SHARED); - for (index = 0; index < arrayP->numProcs; index++) + while (VirtualTransactionIdIsValid(*vxids)) { - int pgprocno = arrayP->pgprocnos[index]; - volatile PGXACT *pgxact = &allPgXact[pgprocno]; - TransactionId pxid; - - /* Fetch xid just once - see GetNewTransactionId */ - pxid = pgxact->xid; - - if (pgxact->inCommit && TransactionIdIsValid(pxid)) + for (index = 0; index < arrayP->numProcs; index++) { - int i; + int pgprocno = arrayP->pgprocnos[index]; + volatile PGPROC *proc = &allProcs[pgprocno]; + volatile PGXACT *pgxact = &allPgXact[pgprocno]; + VirtualTransactionId vxid; - for (i = 0; i < nxids; i++) + GET_VXID_FROM_PGPROC(vxid, *proc); + if (VirtualTransactionIdIsValid(vxid)) { - if (xids[i] == pxid) + if (VirtualTransactionIdEquals(vxid, *vxids) && + pgxact->delayChkpt) { result = true; break; } } - if (result) - break; } + + if (result) + break; + + /* The virtual transaction is gone now, wait for the next one */ + vxids++; } LWLockRelease(ProcArrayLock); diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 429525b6eacf989f34b00ae4b2b3286ff60a21b9..41af7924c0d9e5f38e381d13fccc87c8a66acb48 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -350,7 +350,7 @@ InitProcess(void) MyProc->backendId = InvalidBackendId; MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; - MyPgXact->inCommit = false; + MyPgXact->delayChkpt = false; MyPgXact->vacuumFlags = 0; /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ if (IsAutoVacuumWorkerProcess()) @@ -516,7 +516,7 @@ InitAuxiliaryProcess(void) MyProc->backendId = InvalidBackendId; MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; - MyPgXact->inCommit = false; + MyPgXact->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 e10aafe99e43d19d2346e5a43b240ffa339808dc..686ac486574054a3ada70cd01ee89e48f779c05d 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -168,7 +168,8 @@ typedef struct PGXACT uint8 vacuumFlags; /* vacuum-related flags, see above */ bool overflowed; - bool inCommit; /* true if within commit critical section */ + bool delayChkpt; /* true if this proc delays checkpoint start */ + /* previously called InCommit */ uint8 nxids; } PGXACT; diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h index 5b4cab926f58dda816b6b9dfa9b93fedb6e33fce..9933dad63583e6a6436e37fe6623424b3d45350c 100644 --- a/src/include/storage/procarray.h +++ b/src/include/storage/procarray.h @@ -52,8 +52,8 @@ extern bool TransactionIdIsActive(TransactionId xid); extern TransactionId GetOldestXmin(bool allDbs, bool ignoreVacuum); extern TransactionId GetOldestActiveTransactionId(void); -extern int GetTransactionsInCommit(TransactionId **xids_p); -extern bool HaveTransactionsInCommit(TransactionId *xids, int nxids); +extern VirtualTransactionId *GetVirtualXIDsDelayingChkpt(int *nvxids); +extern bool HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids); extern PGPROC *BackendPidGetProc(int pid); extern int BackendXidGetPid(TransactionId xid);