Horton

Re: Conflict detection for update_deleted in logical replication

shveta malik <shveta.malik@gmail.com>

View thread View on postgr.es
From: shveta malik <shveta.malik@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, vignesh C <vignesh21@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, shveta malik <shveta.malik@gmail.com>
Date: 2025-08-08T03:17:11Z
Lists: pgsql-hackers 
On Thu, Aug 7, 2025 at 6:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 4, 2025 at 3:11 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Aug 4, 2025 at 11:46 AM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > 7)
> > > Shall we rename 'max_conflict_retention_duration' to
> > > 'max_conflict_info_retention_duration' as the latter one is more
> > > clear?
> > >
> >
> > Before bikeshedding on the name of this option, I would like us to
> > once again consider whether we should provide this option at
> > subscription-level or GUC?
> >
> > The rationale behind considering it as a subscription option is that
> > the different subscriptions may have different requirements for dead
> > tuple retention which means that for some particular subscription, the
> > workload may not be always high which means that even if temporarily
> > the lag_duration (of apply) has exceeded the new option's value, it
> > should become okay. So, in such a case users may not want to configure
> > max_conflict_retention_duration for a subscription which would
> > otherwise lead to stop detection of update_deleted conflict for that
> > subscription.
>
> Yes valid point, and it's also possible that for some subscription
> user is okay to not retain dead tuple if it crosses certain duration
> OTOH for some subscription it is too critical to retain dead tuple
> even if user has to take some performance hit, so might want to have
> higher threshold for those slots.
>
> > The other point is that it is only related to the retain_dead_tuples
> > option of the subscription, so providing this new option at the same
> > level would appear consistent.
>
> Yes that's a valid argument, because if the user is setting retain
> dead tuples for subscription then only they need to consider setting
> duration.
>
> > I remember that previously Sawada-San has advocated it to provide as
> > GUC but I think the recent tests suggest that users should define
> > pub-sub topology carefuly to enable retain_dead_tuples option as even
> > mentioned in docs[2], so, it is worth considering to provide it at
> > subscription-level.
>
> IMHO, it should be fine to provide the subscription option first and
> if we see complaints about inconvenience we may consider GUC as well
> in the future.

+1

thanks
Shveta

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.