Horton

RE: Conflict detection for update_deleted in logical replication

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread View on postgr.es
From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.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>, Dilip Kumar <dilipbalaut@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2025-07-25T11:08:46Z
Lists: pgsql-hackers 
On Thursday, July 24, 2025 11:42 AM shveta malik <shveta.malik@gmail.com> wrote:
> 
> On Wed, Jul 23, 2025 at 12:53 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On Wednesday, July 23, 2025 12:08 PM Amit Kapila
> <amit.kapila16@gmail.com> wrote:
> > >
> > > On Wed, Jul 23, 2025 at 3:51 AM Masahiko Sawada
> > > <sawada.mshk@gmail.com> wrote:
> > > >
> > > > I've reviewed the 0001 patch and it looks good to me.
> > > >
> > >
> > > Thanks, I have pushed the 0001 patch.
> >
> > Thanks for pushing. I have rebased the remaining patches.

Thanks for the comments!

> >
> > I have reordered the patches to prioritize the detection of
> > update_deleted as the initial patch. This can give us more time to
> > consider the new GUC, since the performance-related aspects have been
> documented.
> >
> 
> 2)
> +               if (MySubscription->retaindeadtuples &&
> +                       FindMostRecentlyDeletedTupleInfo(localrel,
> + remoteslot,
> +
>                   &conflicttuple.xmin,
> +
>                   &conflicttuple.origin,
> +
>                   &conflicttuple.ts) &&
> +                       conflicttuple.origin != replorigin_session_origin)
> +                       type = CT_UPDATE_DELETED;
> +               else
> +                       type = CT_UPDATE_MISSING;
> 
> Shall the conflict be detected as update_deleted irrespective of origin?

According to the discussion[1], I kept the current behavior.

> 
> 
> 5)
> monitoring.sgml:
> +      <para>
> +       Number of times the tuple to be updated was deleted by another origin
> +       during the application of changes. See <xref
> linkend="conflict-update-deleted"/>
> +       for details about this conflict.
> +      </para></entry>
> 
> Here we are using the term 'by another origin', while in the rest of the doc (see
> confl_update_origin_differs, confl_delete_origin_differs) we use the term 'by
> another source'. Shall we keep it the same?
> OTOH, I think using 'origin' is better but the rest of the page  is using source.
> So perhaps changing source to origin everywhere is better. Thoughts?
> This can be changed if needed once we decide on point 2 above.

Yes, origin may be better. But for now, I have changed to 'source' here to be
consistent with the descriptions around it, and we can improve it in a separate
patch if needed.

Other comments have been addressed in the V53 patch set.

[1] https://www.postgresql.org/message-id/CAA4eK1L09u_A0HFRydA4xc%3DHpPkCh%2B7h-%2B_WRhKw1Cksp5_5zQ%40mail.gmail.com

Best Regards,
Hou zj

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.