RE: Conflict detection for update_deleted in logical replication

On Monday, September 15, 2025 12:55 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> One concern:
>
> if (should_stop_conflict_info_retention(rdt_data))
> + {
> + /*
> + * Stop retention if not yet. Otherwise, reset to the initial phase to
> + * retry all phases. This is required to recalculate the current wait
> + * time and resume retention if the time falls within
> + * max_retention_duration.
> + */
> + if (MySubscription->retentionactive)
> + rdt_data->phase = RDT_STOP_CONFLICT_INFO_RETENTION;
> + else
> + reset_retention_data_fields(rdt_data);
> +
> return;
> + }
>
> Instead of above code changes, shall we have:
>
> if (should_stop_conflict_info_retention(rdt_data))
> rdt_data->phase = RDT_STOP_CONFLICT_INFO_RETENTION;
> (always)
>
> And then stop_conflict_info_retention() should have these checks:
>
> if (MySubscription->retentionactive)
> {
> ...update flag and perform stop (current functionality)
> }
> else
> {
>
> Assert(!TransactionIdIsValid(MyLogicalRepWorker->oldest_nonremovable_xi
> d));
> reset_retention_data_fields(rdt_data);
> }

Thanks for the suggestion. I refactored the codes according to this.

Here is V65 patch which also addressed Amit's comments[1].

Best Regards,
Hou zj