On Sat, Jul 19, 2025 at 3:01 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Fri, Jul 18, 2025 at 5:03 AM Zhijie Hou (Fujitsu) > <houzj.f...@fujitsu.com> wrote: > > > > Here are some review comments and questions: > > > --- > + if (inCommitOnly && > + (proc->delayChkptFlags & DELAY_CHKPT_IN_COMMIT) == 0) > + continue; > + > > I've not verified yet but is it possible that we exclude XIDs of > processes who are running on other databases? >
I can't see how, even the comments atop function says: " We look at all databases, though there is no need to include WALSender since this has no effect on hot standby conflicts." which indicate that it shouldn't exlude XIDs of procs who are running on other databases. > --- > Regarding the option name we've discussed: > > > > The new parameter name "retain_conflict_info" sounds to me like we keep > > > the > > > conflict information somewhere that has expired at some time such as how > > > many times insert_exists or update_origin_differs happened. How about > > > choosing a name that indicates retain dead tuples more explicitly for > > > example > > > retain_dead_tuples? > > > > We considered the name you suggested, but we wanted to convey that this > > option > > not only retains dead tuples but also preserves commit timestamps and origin > > data for conflict detection, hence we opted for a more general name. Do you > > have better suggestions? > > I think the commit timestamp and origin data would be retained as a > result of retaining dead tuples. While such a general name could > convey more than retaining dead tuples, I'm concerned that it could be > ambiguous what exactly to retain by the subscription. How about the > following names or something along those lines? > > - retain_dead_tuples_for_conflict > - delay_vacuum_for_conflict > - keep_dead_tuples_for_conflict > Among these, the first option is better but I think it is better to name it just retain_dead_tuples. The explanation of the option will explain its use. It is similar to other options like binary or streaming. We are not naming them like request_data_binary_format to make the meaning apparent. There is a value in keeping names succinct. -- With Regards, Amit Kapila.
