Re: Conflict detection for update_deleted in logical replication

On Mon, Jul 21, 2025 at 9:30 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Sat, Jul 19, 2025 at 10:32 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Sat, Jul 19, 2025 at 3:01 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Jul 18, 2025 at 5:03 AM Zhijie Hou (Fujitsu)
> > > <houzj(dot)fnst(at)fujitsu(dot)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.
>

It seems unnecessary to track transactions on other databases, as they
won't be replicated to the subscriber.
So, a new parameter 'allDbs' is introduced to control the filtering of
transactions from other databases.

Attached updated V51 patch.
Thank you Hou-san for updating the patch for this change.

--
Thanks,
Nisha