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

On Wed, Mar 6, 2024 at 2:47 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
>
> Thanks. v8-0001 is how it looks. Please see the v8 patch set with this change.
>

@@ -1629,6 +1634,20 @@
InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
  }
  }
  break;
+ case RS_INVAL_INACTIVE_TIMEOUT:
+ if (s->data.last_inactive_at > 0)
+ {
+ TimestampTz now;
+
+ Assert(s->data.persistency == RS_PERSISTENT);
+ Assert(s->active_pid == 0);
+
+ now = GetCurrentTimestamp();
+ if (TimestampDifferenceExceeds(s->data.last_inactive_at, now,
+    inactive_replication_slot_timeout * 1000))

You might want to consider its interaction with sync slots on standby.
Say, there is no activity on slots in terms of processing the changes
for slots. Now, we won't perform sync of such slots on standby showing
them inactive as per your new criteria where as same slots could still
be valid on primary as the walsender is still active. This may be more
of a theoretical point as in running system there will probably be
some activity but I think this needs some thougths.


--
With Regards,
Amit Kapila.