Re: pgsql: Preserve conflict-relevant data during logical replication.

On Wed, Jul 23, 2025 at 11:44 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> The fix looks good to me. I'll push your patch in sometime.

The tests in this patch are insufficient to prove that this logic
works properly. I tried with this patch:

diff --git a/src/backend/access/transam/twophase.c
b/src/backend/access/transam/twophase.c
index 7918176fc58..7c1d0ef07b5 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -2325,6 +2325,16 @@ RecordTransactionCommitPrepared(TransactionId xid,
TimestampTz committs;
bool replorigin;

+ /*
+ * 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 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 comments atop worker.c.
+ */
+ committs = GetCurrentTimestamp();
+
/*
* Are we using the replication origins feature? Or, in other words, are
* we replaying remote actions?
@@ -2344,16 +2354,6 @@ RecordTransactionCommitPrepared(TransactionId xid,
*/
pg_write_barrier();

- /*
- * 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 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 comments atop worker.c.
- */
- committs = GetCurrentTimestamp();
-
/*
* Emit the XLOG commit record. Note that we mark 2PC commits as
* potentially having AccessExclusiveLocks since we don't know whether or

If it is in fact important to acquire the commit timestamp after
setting delayChkptFlags, you'd hope this would lead to a test failure,
but it doesn't for me. I understand it probably requires an injection
point to be certain of hitting the race condition, but I think that
would be worth doing. Otherwise, if something gets broken here by
accident, it might be a long time before anyone notices.

--
Robert Haas
EDB: http://www.enterprisedb.com