Re: Follow-up review items for update_deleted

On Wed, Sep 2, 2026 at 10:00 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Nisha helped perform a thorough review of the update_deleted feature using
> Claude and shared the findings with me off-list. After some analysis and
> discussion, we identified a few items worth addressing (fixes are
> attached (0001-0004)). Please see the details below:
>
> 1. Slot xmin backward movement with multiple subscriptions
>

I'll review this.

>
> See 0001 for the fix and test.
>
> 2. Premature slot advancement with asynchronous commit
>
...
>
> 3
>
> Missing trailing periods to update_deleted conflict DETAILs. Simple fix in 0004.
>

The fix for 2 and 3 looks good to me, so I have pushed those after
changing the comments slightly.

>
> --
> Other items (for recording only)
> --
>
> The following items were also reported but don't seem worth changing at this
> point. They can be revisited later if needed - sharing them here for reference.
>
> 4
>
> With track_commit_timestamp off and retain_dead_tuples on,
> FindDeletedTupleInLocalRel() returns false unconditionally and silently, while
> dead tuples are still retained.
>
> This is not considered as a bug, as the documentation clearly states that
> update_deleted requires track_commit_timestamp to be enabled, and we emit a
> WARNING when retain_dead_tuples is enabled without it. If needed, we could
> improve this by stopping retention when track_commit_timestamp is disabled, but
> disabling track_commit_timestamp while retain_dead_tuples is enabled seems like
> an extremely unlikely user behavior. So would be better to wait for user
> feedback before taking any action.
>
> 5
>
> max_retention_duration does not work for a disabled subscription or a
> keep-failing worker, meaning retention cannot be stopped using this option in
> those cases.
>
> This is documented behavior (and user can disable the retain_dead_tuples
> manually):
>
> > This option is effective only when retain_dead_tuples is enabled and the apply
> > worker associated with the subscription is active.

Both 4 and 5 are documented but will it be better to provide this
information explicitly, say via Logging when the conflcit_slot is not
getting advanced due to any of these reasons?

--
With Regards,
Amit Kapila.