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

On Fri, Mar 29, 2024 at 11:49 AM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> On Fri, Mar 29, 2024 at 09:39:31AM +0530, Amit Kapila wrote:
> >
> > Commit message states: "why we can't just update inactive_since for
> > synced slots on the standby with the value received from remote slot
> > on the primary. This is consistent with any other slot parameter i.e.
> > all of them are synced from the primary."
> >
> > The inactive_since is not consistent with other slot parameters which
> > we copy. We don't perform anything related to those other parameters
> > like say two_phase phase which can change that property. However, we
> > do acquire the slot, advance the slot (as per recent discussion [1]),
> > and release it. Since these operations can impact inactive_since, it
> > seems to me that inactive_since is not the same as other parameters.
> > It can have a different value than the primary. Why would anyone want
> > to know the value of inactive_since from primary after the standby is
> > promoted?
>
> I think it can be useful "before" it is promoted and in case the primary is down.
>

It is not clear to me what is user going to do by checking the
inactivity time for slots when the corresponding server is down. I
thought the idea was to check such slots and see if they need to be
dropped or enabled again to avoid excessive disk usage, etc.

> I agree that tracking the activity time of a synced slot can be useful, why
> not creating a dedicated field for that purpose (and keep inactive_since a
> perfect "copy" of the primary)?
>

We can have a separate field for this but not sure if it is worth it.

> > Now, the other concern is that calling GetCurrentTimestamp()
> > could be costly when the values for the slot are not going to be
> > updated but if that happens we can optimize such that before acquiring
> > the slot we can have some minimal pre-checks to ensure whether we need
> > to update the slot or not.
>
> Right, but for a very active slot it is likely that we call GetCurrentTimestamp()
> during almost each sync cycle.
>

True, but if we have to save a slot to disk each time to persist the
changes (for an active slot) then probably GetCurrentTimestamp()
shouldn't be costly enough to matter.

--
With Regards,
Amit Kapila.