On Fri, Jan 23, 2026 at 7:33 AM Zhijie Hou (Fujitsu) <[email protected]> wrote: > > This patch cannot be applied cleanly on backbranches, I can prepare patches > for > those once the main patch is stable. >
Some comments: 1. + /* + * Determine the minimum non-removable LSN by comparing the redo pointer + * with the minimum slot LSN. + */ + min_safe_lsn = GetRedoRecPtr(); + slot_min_lsn = XLogGetReplicationSlotMinimumLSN(); Can we expand these comments a bit to state why we need both RedoRecPtr and slot's minimum LSN? 2. +# Verify that while syncing a slot to the standby server, if the WAL before the +# remote restart_lsn is at risk of being removed by a checkpoint, the slot +# cannot be synced. Otherwise, even if the slot syncing succeeds, it may be +# immediately invalidated by the checkpoint. +my $primary = $node; This comment atop the testcase is not very clear. Because, it is testing that the slot is synced and is not invalidated. How about: "Verify that the synchronized slots won't be invalidated immediately after synchronization in the presence of a concurrent checkpoint."? 3. +# Increase the log_min_messages setting to DEBUG2 on both the standby and +# primary to debug test failures, if any. +my $connstr_1 = $primary->connstr; Do we need this DEBUG2? I don't think we should add too many DEBUG2 tests as it increases Log volume. -- With Regards, Amit Kapila.
