diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 41fb4fc5025..ea508542745 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -2329,9 +2329,9 @@ RecordTransactionCommitPrepared(TransactionId xid, /* * Note it is important to set committs value after marking ourselves as * in the commit critical section (DELAY_CHKPT_IN_COMMIT). This is because - * we want to ensure all such transactions are finished before we allow - * the logical replication client to advance its xid which is used to hold - * back dead rows for conflict detection. See + * we want to ensure all transactions that have acquired commit timestamp + * are finished before we allow the logical replication client to advance + * its xid which is used to hold back dead rows for conflict detection. See * maybe_advance_nonremovable_xid. */ committs = GetCurrentTimestamp(); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index dfc5108da3c..6e1dc76744f 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -1431,6 +1431,10 @@ RecordTransactionCommit(void) * 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. + * + * Note, it is important to get the commit timestamp after marking the + * transaction in the commit critical section. See + * RecordTransactionCommitPrepared. */ Assert((MyProc->delayChkptFlags & DELAY_CHKPT_IN_COMMIT) == 0); START_CRIT_SECTION(); diff --git a/src/include/replication/worker_internal.h b/src/include/replication/worker_internal.h index 0fdf49a1938..7d1e2096232 100644 --- a/src/include/replication/worker_internal.h +++ b/src/include/replication/worker_internal.h @@ -99,7 +99,9 @@ typedef struct LogicalRepWorker * conditions. Such scenarios might occur if the replication slot is not * yet created by the launcher while the apply worker has already * initialized this field. During this period, a transaction ID wraparound - * could falsely make this ID appear as if it originates from the future. + * could falsely make this ID appear as if it originates from the future + * w.r.t the transaction ID stored in the slot maintained by launcher. See + * advance_conflict_slot_xmin. */ FullTransactionId oldest_nonremovable_xid; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 65ecf3280fb..c6f5ebceefd 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -133,10 +133,10 @@ extern PGDLLIMPORT int FastPathLockGroupsPerBackend; * * Setting DELAY_CHKPT_IN_COMMIT is similar to setting DELAY_CHKPT_START, but * it explicitly indicates that the reason for delaying the checkpoint is due - * to a transaction being within a critical commit section. We need this to - * ensure all such transactions are finished before we allow the logical - * replication client to advance its xid which is used to hold back dead rows - * for conflict detection. + * to a transaction being within a critical commit section. We need this new + * flag to ensure all the transactions that have acquired commit timestamp are + * finished before we allow the logical replication client to advance its xid + * which is used to hold back dead rows for conflict detection. */ #define DELAY_CHKPT_START (1<<0) #define DELAY_CHKPT_COMPLETE (1<<1)