Horton

Re: Introduce XID age and inactive timeout based replication slot invalidation

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Peter Smith <smithpb2250@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, shveta malik <shveta.malik@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@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>
Date: 2025-02-05T04:59:52Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add a test for commit ac0e33136a using the injection point.

  2. Invalidate inactive replication slots.

  3. Fix incorrect slot type in BuildTupleHashTableExt

  4. Allow synced slots to have their inactive_since.

  5. Change last_inactive_time to inactive_since in pg_replication_slots.

  6. Track last_inactive_time in pg_replication_slots.

  7. Track invalidation_reason in pg_replication_slots.

  8. Add option force_initdb to PostgreSQL::Test::Cluster:init()

  9. Add a failover option to subscriptions.

  10. Allow setting failover property in the replication command.

  11. Allow to enable failover property for replication slots via SQL API.

  12. Track conflict_reason in pg_replication_slots.

  13. Log messages for replication slot acquisition and release.

  14. Remove vacuum_defer_cleanup_age

  15. Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids

  16. meson: Add initial version of meson based build system

On Tue, 4 Feb 2025 at 19:56, Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Here is v69 patch set addressing above and Kuroda-san's comments in [1].

Few minor suggestions:
1) In the slot invalidation reporting below:
+               case RS_INVAL_IDLE_TIMEOUT:
+                       Assert(inactive_since > 0);
+
+                       /* translator: second %s is a GUC variable name */
+                       appendStringInfo(&err_detail, _("The slot's
idle time %s exceeds the configured \"%s\" duration."),
+
timestamptz_to_str(inactive_since),
+
"idle_replication_slot_timeout");
+                       /* translator: %s is a GUC variable name */
+                       appendStringInfo(&err_hint, _("You might need
to increase \"%s\"."),
+
"idle_replication_slot_timeout");

It is logged like:
2025-02-05 10:04:11.616 IST [330567] DETAIL:  The slot's idle time
2025-02-05 10:02:49.131631+05:30 exceeds the configured
"idle_replication_slot_timeout" duration.

Here even though we tell idle time, we are logging the inactive_since
value which kind of gives a wrong meaning.

How about we change it to:
The slot has been inactive since 2025-02-05 10:02:49.131631+05:30,
which exceeds the configured "idle_replication_slot_timeout" duration.

2) Here we have mentioned about invalidation happens only for a)
released slots b) inactive slots replication slots c) slot where
communication between pub and sub is down
+                * XXX: Slot invalidation due to 'idle_timeout' applies only to
+                * released slots, and is based on the
'idle_replication_slot_timeout'
+                * GUC. Active slots currently in use for replication
are excluded to
+                * prevent accidental invalidation. Slots where
communication between
+                * the publisher and subscriber is down are also
excluded, as they are
+                * managed by the 'wal_sender_timeout'.
+                */
+               InvalidateObsoleteReplicationSlots(RS_INVAL_IDLE_TIMEOUT,
+
            0,
+
            InvalidOid,
+
            InvalidTransactionId);
a) Can we include about slots which does not reserve WAL are also not
considered.
b) Could we present this in a bullet-point format like the following:
+                * XXX: Slot invalidation due to 'idle_timeout' applies only to:
+                * 1) released slots, and is based on the
'idle_replication_slot_timeout'
+                * GUC. 2) Active slots currently in use for
replication are excluded to
+                * prevent accidental invalidation. 3) Slots where
communication between
+                * the publisher and subscriber is down are also
excluded, as they are
+                * managed by the 'wal_sender_timeout'.
+                */
c) While I was initially reviewing the patch I also had the similar
thoughts on my mind, if we could mention the one like "Slots where
communication between the publisher and subscriber is down are also
excluded, as they are managed by the 'wal_sender_timeout'" in the
documentation it might be good.

Regards,
Vignesh