On Fri, Aug 1, 2025 at 3:58 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> 4.
> + /*
> + * Instead of invoking GetOldestNonRemovableTransactionId() for conflict
> + * detection, we use the conflict detection slot.xmin. This value will be
> + * greater than or equal to the other threshold and provides a more direct
> + * and efficient way to identify recently deleted dead tuples relevant to
> + * the conflict detection. The oldest_nonremovable_xid is not used here,
> + * as it is maintained only by the leader apply worker and unavailable to
> + * table sync and parallel apply workers.
> + */
> + slot = SearchNamedReplicationSlot(CONFLICT_DETECTION_SLOT, true);
>
> This comment seems a bit confusing to me, Isn't it actually correct to
> just use the "conflict detection slot.xmin" even without any other
> reasoning?
>
But it is *not* wrong to use even GetOldestNonRemovableTransactionId()
because it will anyway consider conflict detection slot's xmin.
However, the value returned by that function could be much older, so
slot's xmin is a better choice. Similarly, it is sufficient to use
oldest_nonremovable_xid value of apply worker and ideally would be
better than slot's xmin because it could give update_deleted in fewer
cases, however, we can't use that because of reasons mentioned in the
comments. Do you think this comment needs improvement for clarity and
if so, do you have any proposal?
--
With Regards,
Amit Kapila.
