RE: Conflict detection for update_deleted in logical replication

On Friday, September 12, 2025 4:43 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Fri, Sep 12, 2025 at 8:55 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > I agree. Here is a V73 patch that will restart the worker if the
> > retention resumes. I also addressed other comments posted by Amit[1].
> >
> 
> Few comments:
> ============
> * In adjust_xid_advance_interval(), for the case when retention is not active,
> we still cap the interval by wal_receiver_status_interval. Is that required or do
> we let it go till 3 minutes? We can add a new else if loop to keep the code clear,
> if you agree with this.

I agree we can let it go till 3 mins, and changed the patch as suggested.

> 
> *
> + /*
> + * Resume retention immediately if required. (See
> + * should_resume_retention_immediately() for details).
> + */
> + if (should_resume_retention_immediately(rdt_data, status_received))
> + rdt_data->phase = RDT_RESUME_CONFLICT_INFO_RETENTION;
> 
> Is this optimization for the case when we are in stop_phase or after stop_phase
> and someone has changed maxretention to 0? If so, I don't think it is worth
> optimizing for such a rare case at the cost of making code difficult to
> understand.

Agreed. I removed this in V74.

> 
> Apart from this, I have changed a few comments in the attached.

Thanks for the patch, it looks good to me. I have merged it in V74.

Best Regards,
Hou zj