On Friday, September 12, 2025 4:43 PM Amit Kapila <[email protected]> wrote: > > On Fri, Sep 12, 2025 at 8:55 AM Zhijie Hou (Fujitsu) <[email protected]> > 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
