RE: Conflict detection for update_deleted in logical replication

On Tue, May 20, 2025 at 6:30 PM shveta malik wrote:
>
> Few more comments mostly for patch001:

Thanks for the comments!

>
> 4)
> For this feature, since we are only interested in remote UPDATEs happening
> concurrently, so shall we ask primary to provide oldest "UPDATE"
> transaction-id in commit-phase rather than any operation's transaction-id?
> This may avoid unnecessarily waiting and pinging at subscriber's end in order
> to advance oldest_nonremovable-xid.
> Thoughts?

It is possible, but considering the potential complexity/cost to track UPDATE
operations in top-level and sub-transactions, coupled with its limited benefit
for workloads featuring frequent UPDATEs on publishers such as observed during
TPC-B performance tests, I have opted to document this possibility in comments
instead of implementing it in the patch set.

>
> 5)
> +
> +/*
> + * GetOldestTransactionIdInCommit()
> + *
> + * Similar to GetOldestActiveTransactionId but returns the oldest
> transaction ID
> + * that is currently in the commit phase.
> + */
> +TransactionId
> +GetOldestTransactionIdInCommit(void)
>
> If there is no transaction currently in 'commit' phase, this function will then
> return the next transaction-id. Please mention this in the comments. I think the
> doc 'protocol-replication.html' should also be updated for the same.

I added this info in the doc. But since we have merged this function with
GetOldestActiveTransactionId() which has the same behavior, so I am
not adding more code comments for the existing function.

Best Regards,
Hou zj