On Fri, Mar 29, 2024 at 11:49 AM Bertrand Drouvot <bertranddrouvot...@gmail.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.
