On Sun, Jul 20, 2025 at 9:00 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > 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.
Right. > 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. Why do we need to include all XIDs even in the cases called from ProcessStandbyPSRequestMessage()? I guess that there is no chance that the changes happening on other (non-subscribed) databases could conflict with something on the subscriber. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
