Horton

Re: Conflict detection for update_deleted in logical replication

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: shveta malik <shveta.malik@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, vignesh C <vignesh21@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>
Date: 2025-09-10T08:39:34Z
Lists: pgsql-hackers 
On Wed, Sep 10, 2025 at 9:08 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Tuesday, September 9, 2025 7:01 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Tue, Sep 9, 2025 at 11:47 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> > wrote:
> >
> > Few other comments:
> > 1.
> > + /*
> > + * Return if the launcher has not initialized oldest_nonremovable_xid.
> > + *
> > + * It might seem feasible to directly check the conflict detection
> > + * slot.xmin instead of relying on the launcher to assign the worker's
> > + * oldest_nonremovable_xid; however, that could lead to a race
> > + condition
> > + * where slot.xmin is set to InvalidTransactionId immediately after the
> > + * check. In such cases, oldest_nonremovable_xid would no longer be
> > + * protected by a replication slot and could become unreliable if a
> > + * wraparound occurs.
> > + */
> > + if (!TransactionIdIsValid(nonremovable_xid))
> > + return;
> >
> > I understand the reason why we get assigned the worker's non_removable_xid
> > from launcher but all this makes the code complex to understand. Isn't there
> > any other way to achieve it? One naïve and inefficient way could be to just
> > restart the worker probably after updating its retentionactive flag. I am not
> > suggesting to make this change but just a brainstorming point.
>
> I'll keep thinking about it, and for now, I've added a comment mentioning
> that rebooting is a simpler solution.
>

Say we have a LW LogicalRepRetentionLock. We acquire it in SHARED mode
as soon as we encounter the first subscription with retain_dead_tuples
set during the traversal of the sublist. We release it only after
updating xmin outside the sublist traversal. We then acquire it in
EXCLUSIVE mode to fetch the resume the retention in worker for the
period till we fetch slot's xmin.

This will close the above race condition but acquiring LWLock while
traversing subscription is not advisable as that will make it
uninterruptible. The other possibility is to use some heavy-weight
lock here, say a lock on pg_subscription catalog but that has a
drawback that it can conflict with DDL operations. Yet another way is
to invent a new lock-type for this.

OTOH, the simple strategy to let apply-worker restart to resume
retention will keep the handling simpler. We do something similar at
the start of apply-worker if we find that some subscription parameter
is changed. I think we need more opinions on this matter.

One other comment:
+ if (!MySubscription->retentionactive)
+ {
+ rdt_data->phase = RDT_RESUME_CONFLICT_INFO_RETENTION;
+ process_rdt_phase_transition(rdt_data, false);
+ return;
+ }

It is better that the caller processes the next phase, otherwise, this
looks a bit ad hoc. Similarly, please check other places.

-- 
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.