Hi, On Fri, Mar 22, 2024 at 01:45:01PM +0530, Bharath Rupireddy wrote: > On Fri, Mar 22, 2024 at 12:39 PM Bertrand Drouvot > <bertranddrouvot...@gmail.com> wrote: > > > > > > Please find the v14-0001 patch for now. > > > > Thanks! > > > > > LGTM. Let's wait for Bertrand to see if he has more comments on 0001 > > > and then I'll push it. > > > > LGTM too. > > Thanks. Here I'm implementing the following:
Thanks! > 0001 Track invalidation_reason in pg_replication_slots > 0002 Track last_inactive_at in pg_replication_slots > 0003 Allow setting inactive_timeout for replication slots via SQL API > 0004 Introduce new SQL funtion pg_alter_replication_slot > 0005 Allow setting inactive_timeout in the replication command > 0006 Add inactive_timeout based replication slot invalidation > > 1. Keep it last_inactive_at as a shared memory variable, but always > set it at restart if the slot's inactive_timeout has non-zero value > and reset it as soon as someone acquires that slot so that if the slot > doesn't get acquired till inactive_timeout, checkpointer will > invalidate the slot. > 4. last_inactive_at should also be set to the current time during slot > creation because if one creates a slot and does nothing with it then > it's the time it starts to be inactive. I did not look at the code yet but just tested the behavior. It works as you describe it but I think this behavior is weird because: - when we create a slot without a timeout then last_inactive_at is set. I think that's fine, but then: - when we restart the engine, then last_inactive_at is gone (as timeout is not set). I think last_inactive_at should be set also at engine restart even if there is no timeout. I don't think we should link both. Changing my mind here on this subject due to the testing. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
