On Sat, Jul 19, 2025 at 10:32 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > 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. >
I think I misunderstood your question. You were asking if possible, we should exclude XIDs of processes running on other databases in the above check as for our purpose, we don't need those. If so, I agree with you, we don't need XIDs of other databases as logical WALSender will anyway won't process transactions in other databases, so we can exclude those. The function GetOldestActiveTransactionId() is called from two places in patch get_candidate_xid() and ProcessStandbyPSRequestMessage(). We don't need to care for XIDs in other databases at both places but care for Commit_Critical_Section_Phase when called from ProcessStandbyPSRequestMessage(). So, we probably need two parameters to distinguish those cases. -- With Regards, Amit Kapila.
