Re: logical: fix recomputation required LSN on restart_lsn-only advancement

On Tue, Apr 21, 2026 at 10:16 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:

> Hi,
>
> While reading logical replication code, I found an issue in
> LogicalConfirmReceivedLocation().
>
> In LogicalConfirmReceivedLocation(), updated_restart is tracked
> independently from updated_xmin, and the slot is marked dirty and saved
> when either one changed. But after that,
> ReplicationSlotsComputeRequiredLSN() is still only called inside "if
> (updated_xmin)”.
>
> So for the restart-only case:
>
> * updated_restart = true
> * updated_xmin = false
> * ReplicationSlotSave() runs
> * ReplicationSlotsComputeRequiredLSN() does not run because updated_xmin
> is false
>
> That means the global retention point managed by
> XLogSetReplicationSlotMinimumLSN() can stay stale until some later
> unrelated event recomputes it. Since ReplicationSlotsComputeRequiredLSN()
> derives the global minimum from slot restat_lsn, skipping it after a
> restart-only advance can retain excess WAL and may lead to WAL bloat.
>
> This patch fixes the problem by moving
> ReplicationSlotsComputeRequiredLSN() under “if (updated_restart)”.
>
> Looks like this issue has been there for a long time, so if this analysis
> is correct, it may also be worth back-patching.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
I think this change is reasonable.
This makes the recomputation condition match the state it actually depends
on.

Regards,
Xunqi Hu