Re: Conflict detection for update_deleted in logical replication

On Mon, Jun 30, 2025 at 6:59 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Mon, Jun 30, 2025 at 7:22 PM Amit Kapila wrote:
> >

I was looking at 0001, it mostly looks fine to me except this one
case. So here we need to ensure that commits must be acquired after
marking the flag, don't you think we need to ensure strict statement
ordering using memory barrier, or we think it's not required and if so
why?

RecordTransactionCommitPrepared()
{
..
+ MyProc->delayChkptFlags |= DELAY_CHKPT_IN_COMMIT;
+
+ /*
+ * 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 maybe_advance_nonremovable_xid.
+ */
+ committs = GetCurrentTimestamp();
}

--
Regards,
Dilip Kumar
Google