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: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.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>, shveta malik <shveta.malik@gmail.com>
Date: 2025-07-28T11:43:08Z
Lists: pgsql-hackers 
On Mon, Jul 28, 2025 at 4:38 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Fri, Jul 25, 2025 at 4:38 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> >
> > The V53-0001 also includes Shveta's comments in [1].
> >
>
> Thanks, I have not yet completed the review, but please find a few
> comments on 001:
>
> 1)
> IsIndexUsableForFindingDeletedTuple()
> We first have:
> + /*
> + * A frozen transaction ID indicates that it must fall behind the conflict
> + * detection slot.xmin.
> + */
> + if (HeapTupleHeaderXminFrozen(index_tuple->t_data))
> + return true;
>
> thent his:
> + index_xmin = HeapTupleHeaderGetRawXmin(index_tuple->t_data);
>
> Shall we use HeapTupleHeaderGetXmin() instead of above 2? We can check
> if xid returned by HeapTupleHeaderGetXmin() is FrozenTransactionId or
> normal one and then do further processing.
>
>
> 2)
> Both FindRecentlyDeletedTupleInfoByIndex and
> FindRecentlyDeletedTupleInfoSeq() has:
> + /* Exit early if the commit timestamp data is not available */
> + if (!track_commit_timestamp)
> + return false;
>
> We shall either move this check to FindDeletedTupleInLocalRel() or in
> the caller of FindDeletedTupleInLocalRel() where we check
> 'MySubscription->retaindeadtuples'. Moving to the caller of
> FindDeletedTupleInLocalRel() looks better as there is no need to call
> FindDeletedTupleInLocalRel itself if pre-conditions are not met.
>
>
> 3)
> FindRecentlyDeletedTupleInfoSeq():
> + * during change applications. While this approach may be slow on large tables,
> + * it is considered acceptable because it is only used in rare conflict cases
> + * where the target row for an update cannot be found.
> + */
> Shall we extend the last line to:
> where the target row for an update cannot be found and index scan can
> not be used.
>
>
> 4)
> catalogs.sgml:
> +       If true, he detection of <xref linkend="conflict-update-deleted"/> is
> he --> the
>

5)
FindRecentlyDeletedTupleInfoSeq():

+ /* Get the cutoff xmin for HeapTupleSatisfiesVacuum */
+ oldestxmin = GetOldestNonRemovableTransactionId(rel);

Another point is which xid should be used as threshold in
HeapTupleSatisfiesVacuum() to decide if tuple is DEAD or RECENTLY-DEAD
for update_deleted case? Currently we are using
GetOldestNonRemovableTransactionId() but the xid returned here could
be older than pg_conflict_detection's xmin in presence of other
logical slots which have older effective_xmin. Shall we use
pg_conflict_detection's xmin instead as threshold or worker's
oldest_nonremovable_xid?

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.