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? We don't expose slot creation time, no? 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? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com