Re: logical: fix recomputation required LSN on restart_lsn-only advancement - Mailing list pgsql-hackers

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/