On Mon, Feb 3, 2025 at 6:35 PM Shlok Kyal <shlok.kyal....@gmail.com> wrote: > > I reviewed the v66 patch. I have few comments: > > 1. I also feel the default value should be set to '0' as suggested by > Vignesh in 1st point of [1]. >
+1. This will ensure that the idle slots won't be invalidated by default, the same as HEAD. We can change the default value based on user inputs. > 2. Should we allow copying of invalidated slots? > Currently we are able to copy slots which are invalidated: > > postgres=# select slot_name, active, restart_lsn, wal_status, > inactive_since , invalidation_reason from pg_replication_slots; > slot_name | active | restart_lsn | wal_status | > inactive_since | invalidation_reason > -----------+--------+-------------+------------+----------------------------------+--------------------- > test1 | f | 0/16FDDE0 | lost | 2025-02-03 > 18:28:01.802463+05:30 | idle_timeout > (1 row) > > postgres=# select pg_copy_logical_replication_slot('test1', 'test2'); > pg_copy_logical_replication_slot > ---------------------------------- > (test2,0/16FDE18) > (1 row) > > postgres=# select slot_name, active, restart_lsn, wal_status, > inactive_since , invalidation_reason from pg_replication_slots; > slot_name | active | restart_lsn | wal_status | > inactive_since | invalidation_reason > -----------+--------+-------------+------------+----------------------------------+--------------------- > test1 | f | 0/16FDDE0 | lost | 2025-02-03 > 18:28:01.802463+05:30 | idle_timeout > test2 | f | 0/16FDDE0 | reserved | 2025-02-03 > 18:29:53.478023+05:30 | > (2 rows) > Is this related to this patch or the behavior of HEAD? If this behavior is not introduced by this patch then we should discuss this in a separate thread. I couldn't think of why anyone wants to copy the invalid slots, so we should probably prohibit copying invalid slots but that is a matter of separate discussion unless introduced by this patch. -- With Regards, Amit Kapila.