On Tue, Mar 26, 2024 at 4:35 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Tue, Mar 26, 2024 at 4:18 PM shveta malik <shveta.ma...@gmail.com> wrote: > > > > > What about another approach?: inactive_since gives data synced from > > > primary for > > > synced slots and another dedicated field (could be added later...) could > > > represent what you suggest as the other option. > > > > Yes, okay with me. I think there is some confusion here as well. In my > > second approach above, I have not suggested anything related to > > sync-worker. We can think on that later if we really need another > > field which give us sync time. In my second approach, I have tried to > > avoid updating inactive_since for synced slots during sync process. We > > update that field during creation of synced slot so that > > inactive_since reflects correct info even for synced slots (rather > > than copying from primary). Please have a look at my patch and let me > > know your thoughts. I am fine with copying it from primary as well and > > documenting this behaviour. > > I took a look at your patch. > > --- a/src/backend/replication/logical/slotsync.c > +++ b/src/backend/replication/logical/slotsync.c > @@ -628,6 +628,7 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid > remote_dbid) > SpinLockAcquire(&slot->mutex); > slot->effective_catalog_xmin = xmin_horizon; > slot->data.catalog_xmin = xmin_horizon; > + slot->inactive_since = GetCurrentTimestamp(); > SpinLockRelease(&slot->mutex); > > If we just sync inactive_since value for synced slots while in > recovery from the primary, so be it. Why do we need to update it to > the current time when the slot is being created?
If we update inactive_since at synced slot's creation or during restart (skipping setting it during sync), then this time reflects actual 'inactive_since' for that particular synced slot. Isn't that a clear info for the user and in alignment of what the name 'inactive_since' actually suggests? > We don't expose slot > creation time, no? No, we don't. But for synced slot, that is the time since that slot is inactive (unless promoted), so we are exposing inactive_since and not creation time. >Aren't we fine if we just sync the value from > primary and document that fact? After the promotion, we can reset it > to the current time so that it gets its own time. Do you see any > issues with it? Yes, we can do that. But curious to know, do we see any additional benefit of reflecting primary's inactive_since at standby which I might be missing? thanks Shveta