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> <CALDaNm0mTWwg0z4v-sorq08S2CdZmL2s+rh4nHpWeJaBQ2F+mg@mail.gmail.com>
In-Reply-To: <CALDaNm0mTWwg0z4v-sorq08S2CdZmL2s+rh4nHpWeJaBQ2F+mg@mail.gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
Date: Thu, 5 Dec 2024 12:13:40 +1100
Message-ID: <CAHut+Psi-mso-qSyMkHRP8e+psDFFmMp4NZTiL_yrFWidHkGMw@mail.gmail.com>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
To: vignesh C <vignesh21@gmail.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, 
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, shveta malik <shveta.malik@gmail.com>, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@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/CAHut%2BPsi-mso-qSyMkHRP8e%2BpsDFFmMp4NZTiL_yrFWidHkGMw%40mail.gmail.com>
Precedence: bulk

On Wed, Dec 4, 2024 at 9:27=E2=80=AFPM vignesh C <vignesh21@gmail.com> wrot=
e:
>
...
>
> 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 checkpoi=
nt.
> +        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.
>

Hi Vignesh.

I felt that manipulating the checkpoint timing behind the scenes
without the user's consent might be a bit of an overreach.

But there might still be something else we could do:

1. We can add the documentation note like you suggested ("we could
document about this delay in identifying and mention that it could be
triggered by forceful manual checkpoint").

2. We can also detect such delays in the code. When the invalidation
occurs (e.g. code fragment below) we could check if there was some
excessive lag between the slot becoming idle and it being invalidated.
If the lag is too much (whatever "too much" means) we can log a hint
for the user to increase the checkpoint frequency (or whatever else we
might advise them to do).

+ /*
+ * Check if the slot needs to be invalidated due to
+ * replication_slot_inactive_timeout GUC.
+ */
+ if (IsSlotInactiveTimeoutPossible(s) &&
+ TimestampDifferenceExceeds(s->inactive_since, now,
+    replication_slot_inactive_timeout_ms))
+ {
+ invalidation_cause =3D cause;
+ inactive_since =3D s->inactive_since;

pseudo-code:
if (slot invalidation occurred much later after the
replication_slot_inactive_timeout GUC elapsed)
{
  elog(LOG, "This slot was inactive for a period of %s. Slot timeout
invalidation only occurs at a checkpoint so if you want inactive slots
to be invalidated in a more timely manner consider reducing the time
between checkpoints or executing a manual checkpoint.
(replication_slot_inactive_timeout =3D %s; checkpoint_timeout =3D %s,
....)"
}

+ }

=3D=3D=3D=3D=3D=3D
Kind Regards,
Peter Smith.
Fujitsu Australia