On Tue, Mar 26, 2024 at 1:54 PM Bertrand Drouvot
<bertranddrouvot...@gmail.com> wrote:
>
> Hi,
>
> On Tue, Mar 26, 2024 at 01:37:21PM +0530, Amit Kapila wrote:
> > On Tue, Mar 26, 2024 at 1:15 PM Bertrand Drouvot
> > <bertranddrouvot...@gmail.com> wrote:
> > >
> > > 2 ===
> > >
> > > It looks like inactive_since is set to the current timestamp on the
> > > standby
> > > each time the sync worker does a cycle:
> > >
> > > primary:
> > >
> > > postgres=# select slot_name,inactive_since from pg_replication_slots
> > > where failover = 't';
> > > slot_name | inactive_since
> > > -------------+-------------------------------
> > > lsub27_slot | 2024-03-26 07:39:19.745517+00
> > > lsub28_slot | 2024-03-26 07:40:24.953826+00
> > >
> > > standby:
> > >
> > > postgres=# select slot_name,inactive_since from pg_replication_slots
> > > where failover = 't';
> > > slot_name | inactive_since
> > > -------------+-------------------------------
> > > lsub27_slot | 2024-03-26 07:43:56.387324+00
> > > lsub28_slot | 2024-03-26 07:43:56.387338+00
> > >
> > > I don't think that should be the case.
> > >
> >
> > But why? This is exactly what we discussed in another thread where we
> > agreed to update inactive_since even for sync slots.
>
> Hum, I thought we agreed to "sync" it and to "update it to current time"
> only at promotion time.
I think there may have been some misunderstanding here. But now if I
rethink this, I am fine with 'inactive_since' getting synced from
primary to standby. But if we do that, we need to add docs stating
"inactive_since" represents primary's inactivity and not standby's
slots inactivity for synced slots. The reason for this clarification
is that the synced slot might be generated much later, yet
'inactive_since' is synced from the primary, potentially indicating a
time considerably earlier than when the synced slot was actually
created.
Another approach could be that "inactive_since" for synced slot
actually gives its own inactivity data rather than giving primary's
slot data. We update inactive_since on standby only at 3 occasions:
1) at the time of creation of the synced slot.
2) during standby restart.
3) during promotion of standby.
I have attached a sample patch for this idea as.txt file.
I am fine with any of these approaches. One gives data synced from
primary for synced slots, while another gives actual inactivity data
of synced slots.
thanks
Shveta
From 7dcd0e95299263187eb1f03812f8321b2612ee5c Mon Sep 17 00:00:00 2001
From: Shveta Malik <shveta.ma...@gmail.com>
Date: Tue, 26 Mar 2024 14:42:25 +0530
Subject: [PATCH v1] inactive_since for synced slots.
inactive_since is updated for synced slots:
1) at the time of creation of slot.
2) during server restart.
3) during promotion.
---
src/backend/replication/logical/slotsync.c | 1 +
src/backend/replication/slot.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/backend/replication/logical/slotsync.c
b/src/backend/replication/logical/slotsync.c
index bbf9a2c485..6114895dca 100644
--- 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);
ReplicationSlotsComputeRequiredXmin(true);
LWLockRelease(ProcArrayLock);
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index d0a2f440ef..f2a57a14ec 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -628,7 +628,10 @@ retry:
* now.
*/
SpinLockAcquire(&s->mutex);
- s->inactive_since = 0;
+
+ if (!(RecoveryInProgress() && s->data.synced))
+ s->inactive_since = 0;
+
SpinLockRelease(&s->mutex);
if (am_walsender)
@@ -704,14 +707,20 @@ ReplicationSlotRelease(void)
*/
SpinLockAcquire(&slot->mutex);
slot->active_pid = 0;
- slot->inactive_since = now;
+
+ if (!(RecoveryInProgress() && slot->data.synced))
+ slot->inactive_since = now;
+
SpinLockRelease(&slot->mutex);
ConditionVariableBroadcast(&slot->active_cv);
}
else
{
SpinLockAcquire(&slot->mutex);
- slot->inactive_since = now;
+
+ if (!(RecoveryInProgress() && slot->data.synced))
+ slot->inactive_since = now;
+
SpinLockRelease(&slot->mutex);
}
--
2.34.1
