Re: Conflict detection for update_deleted in logical replication

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, shveta malik <shveta.malik@gmail.com>, vignesh C <vignesh21@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>
Date: 2025-07-21T04:00:25Z
Lists: pgsql-hackers
On Sat, Jul 19, 2025 at 10:32 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Jul 19, 2025 at 3:01 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Jul 18, 2025 at 5:03 AM Zhijie Hou (Fujitsu)
> > <houzj.fnst@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.



Commits

  1. Fix intermittent BF failures in 035_conflicts.

  2. Resume conflict-relevant data retention automatically.

  3. Fix intermittent test failure introduced in 6456c6e2c4.

  4. Fix Coverity issue reported in commit a850be2fe.

  5. Add test to prevent premature removal of conflict-relevant data.

  6. Post-commit review fixes for 228c370868.

  7. Add max_retention_duration option to subscriptions.

  8. Detect and report update_deleted conflicts.

  9. Preserve conflict-relevant data during logical replication.

  10. Integrate FullTransactionIds deeper into two-phase code

  11. Improve checks for GUC recovery_target_timeline

  12. Prevent excessive delays before launching new logrep workers.

  13. Keep WAL segments by slot's last saved restart LSN

  14. Rework some code handling pg_subscription data in psql and pg_dump

  15. Use generateClonedIndexStmt to propagate CREATE INDEX to partitions.