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

On Thu, Apr 25, 2024 at 11:11 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Mon, Apr 22, 2024 at 7:21 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > > Please find the attached v35 patch.
> >
> > The documentation says about both 'active' and 'inactive_since'
> > columns of pg_replication_slots say:
> >
> > ---
> > active bool
> > True if this slot is currently actively being used
> >
> > inactive_since timestamptz
> > The time since the slot has become inactive. NULL if the slot is
> > currently being used. Note that for slots on the standby that are
> > being synced from a primary server (whose synced field is true), the
> > inactive_since indicates the last synchronization (see Section 47.2.3)
> > time.
> > ---
> >
> > When reading the description I thought if 'active' is true,
> > 'inactive_since' is NULL, but it doesn't seem to apply for temporary
> > slots.
>
> Right.
>
> > Since we don't reset the active_pid field of temporary slots
> > when the release, the 'active' is still true in the view but
> > 'inactive_since' is not NULL.
>
> Right. inactive_since is reset whenever the temporary slot is acquired
> again within the same backend that created the temporary slot.
>
> > Do you think we need to mention it in
> > the documentation?
>
> I think that's the reason we dropped "active" from the statement. It
> was earlier "NULL if the slot is currently actively being used.". But,
> per Bertrand's comment
> https://www.postgresql.org/message-id/ZehE2IJcsetSJMHC%40ip-10-97-1-34.eu-west-3.compute.internal
> changed it to ""NULL if the slot is currently being used.".
>
> Temporary slots retain the active = true and active_pid = <pid of the
> backend that created it> even when the slot is not being used until
> the lifetime of the backend process. We haven't tied active or
> active_pid flags to inactive_since, doing so now to represent the
> temporary slot behaviour for active and active_pid will confuse users
> more.
>

This is true and it's probably easy for us to understand as we
developed this feature but the same may not be true for others. I
wonder if we can be explicit about the difference of
active/inactive_since by adding something like the following for
inactive_since: Note that this field is not related to the active flag
as temporary slots can remain active till the session ends even when
they are not being used.

Sawada-San, do you have any suggestions on the wording?

>
 As far as the inactive_since of a slot is concerned, it is set
> to 0 when the slot is being used (acquired) and set to current
> timestamp when the slot is not being used (released).
>
> > As for the timeout-based slot invalidation feature, we could end up
> > invalidating the temporary slots even if they are shown as active,
> > which could confuse users. Do we want to somehow deal with it?
>
> Yes. As long as the temporary slot is lying unused holding up
> resources for more than the specified
> replication_slot_inactive_timeout, it is bound to get invalidated.
> This keeps behaviour consistent and less-confusing to the users.
>

Agreed. We may want to add something in the docs for this to avoid
confusion with the active flag.

--
With Regards,
Amit Kapila.