Re: Introduce XID age and inactive timeout based replication slot invalidation

On Mon, Feb 3, 2025 at 6:35 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)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.