On Thursday, January 22, 2026 2:54 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
>
> Thanks for updating the patch. Further comments.
Thanks for the comments.
>
> 01.
> ```
> +#include "access/xlog.h"
> ```
>
> I could build without the inclusion because "replication/logical.h" already
> includes it. Can we remove or we should retain?
Removed.
>
> 02.
> Should we increase checkpoint_timeout for stabilizing tests?
I think we don't need this as concurrent checkpoint won't
cause the slot to be invalidated.
>
> 03.
> To confirm, you've removed the logic that checks the oldest segment and try
> reserving, but it can be removed same as ReplicationSlotReserveWal(), right?
If you meant we can remove the retry logic similar to what 3510ebe did, the
understanding is correct.
> XLogGetOldestSegno() is also not needed anymore because race can't happen
> if standby have never discarded.
Yes
>
> 04.
> ```
> $primary->psql('postgres',
> q{SELECT pg_create_logical_replication_slot('failover_slot',
> 'test_decoding', false, false, true);
> SELECT pg_create_physical_replication_slot('phys_slot');}
> );
> ...
> $primary->psql('postgres', "CHECKPOINT"); ```
>
> I found two lines use `psql()`, but should be `safe_psql()`.
Changed.
>
> 05.
> Per my tests, the issue exists till PG17 and your patch can be backpatched till
> it, right?
This patch cannot be applied cleanly on backbranches, I can prepare patches for
those once the main patch is stable.
Best Regards,
Hou zj
