MIME-Version: 1.0
References: <CALDaNm39dtWHoQej2wd74zYGphA0e54Rx6aTRoCEnMXZ5ysbow@mail.gmail.com>
 <CABdArM45=j=DAFTEZA8p=oqYo-XqRwhaPCdKpciQCJx+jNeTJg@mail.gmail.com>
 <TYAPR01MB5692B7687EE7981AA91BA5B9F5362@TYAPR01MB5692.jpnprd01.prod.outlook.com>
 <CABdArM4N60XShbmxXgz4z8ENQGPOMhXSKzvmskdv8V=mQsYOOA@mail.gmail.com>
In-Reply-To: <CABdArM4N60XShbmxXgz4z8ENQGPOMhXSKzvmskdv8V=mQsYOOA@mail.gmail.com>
From: vignesh C <vignesh21@gmail.com>
Date: Wed, 4 Dec 2024 15:56:53 +0530
Message-ID: <CALDaNm0mTWwg0z4v-sorq08S2CdZmL2s+rh4nHpWeJaBQ2F+mg@mail.gmail.com>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, shveta malik <shveta.malik@gmail.com>, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, 
	Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, 
	Ajin Cherian <itsajin@gmail.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, 
	Masahiko Sawada <sawada.mshk@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CALDaNm0mTWwg0z4v-sorq08S2CdZmL2s%2Brh4nHpWeJaBQ2F%2Bmg%40mail.gmail.com>
Precedence: bulk

On Wed, 4 Dec 2024 at 15:01, Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Tue, Dec 3, 2024 at 1:09=E2=80=AFPM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Nisha,
> >
> > Thanks for updating the patch!
> >
> > > Fixed. It is reasonable to align with other timeout parameters by
> > > using milliseconds as the unit.
> >
> > It looks you just replaced to GUC_UNIT_MS, but the documentation and
> > postgresql.conf.sample has not been changed yet. They should follow cod=
es.
> > Anyway, here are other comments, mostly cosmetic.
> >
>
> Here is v53 patch-set addressing all the comments in [1] and [2].

Currently, replication slots are invalidated based on the
replication_slot_inactive_timeout only during a checkpoint. This means
that if the checkpoint_timeout is set to a higher value than the
replication_slot_inactive_timeout, slot invalidation will occur only
when the checkpoint is triggered. Identifying the invalidation slots
might be slightly delayed in this case. As an alternative, users can
forcefully invalidate inactive slots that have exceeded the
replication_slot_inactive_timeout by forcing a checkpoint. I was
thinking we could suggest this in the documentation.

+       <para>
+        Slot invalidation due to inactive timeout occurs during checkpoint=
.
+        The duration of slot inactivity is calculated using the slot's
+        <link linkend=3D"view-pg-replication-slots">pg_replication_slots</=
link>.<structfield>inactive_since</structfield>
+        value.
+       </para>
+

We could accurately invalidate the slots using the checkpointer
process by calculating the invalidation time based on the active_since
timestamp and the replication_slot_inactive_timeout, and then set the
checkpointer's main wait-latch accordingly for triggering the next
checkpoint. Ideally, a different process handling this task would be
better, but there is currently no dedicated daemon capable of
identifying and managing slots across streaming replication, logical
replication, and other slots used by plugins. Additionally,
overloading the checkpointer with this responsibility may not be
ideal. As an alternative, we could document about this delay in
identifying and mention that it could be triggered by forceful manual
checkpoint.

Regards,
Vignesh