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

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>

To: Nisha Moond <nisha.moond412@gmail.com>

Cc: Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@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-01-13T06:52:04Z

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 →

Add a test for commit ac0e33136a using the injection point.
- 8a695d7998be 18.0 landed
Invalidate inactive replication slots.
- ac0e33136abc 18.0 landed
Fix incorrect slot type in BuildTupleHashTableExt
- d96d1d5152f3 18.0 cited
Allow synced slots to have their inactive_since.
- 6f132ed693b6 17.0 landed
Change last_inactive_time to inactive_since in pg_replication_slots.
- 6d49c8d4b4f4 17.0 landed
Track last_inactive_time in pg_replication_slots.
- a11f330b5584 17.0 landed
Track invalidation_reason in pg_replication_slots.
- 6ae701b4378d 17.0 landed
Add option force_initdb to PostgreSQL::Test::Cluster:init()
- ff9e1e764fcc 17.0 cited
Add a failover option to subscriptions.
- 776621a5e479 17.0 cited
Allow setting failover property in the replication command.
- 73292404370c 17.0 cited
Allow to enable failover property for replication slots via SQL API.
- c393308b69d2 17.0 cited
Track conflict_reason in pg_replication_slots.
- 007693f2a3ac 17.0 cited
Log messages for replication slot acquisition and release.
- 7c3fb505b14e 17.0 cited
Remove vacuum_defer_cleanup_age
- 1118cd37eb61 16.0 cited
Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids
- be504a3e974d 16.0 cited
meson: Add initial version of meson based build system
- e6927270cd18 16.0 cited

On Thu, 2 Jan 2025 at 15:57, Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Thu, Jan 2, 2025 at 8:16 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Hi Nisha,
> >
> > Here are some minor review comments for patch v58-0002.
> >
>
> Thank you for your feedback! Please find the v59 patch set addressing
> all the comments.
> Note: There are no new changes in patch-0001.

Few comments:
1) I felt we should not invalidate the slots for which has no
effective xmin set as they will not be holding the WAL files from
deletion. This can happen when user created slots with
immediately_reserve as false and lsn will be actually reserved only
after the first connection from a streaming replication client:
+static inline bool
+CanInvalidateIdleSlot(ReplicationSlot *s)
+{
+       return (idle_replication_slot_timeout_mins > 0 &&
+                       s->inactive_since > 0 &&
+                       !(RecoveryInProgress() && s->data.synced));
+}

2) We can mention this as 1d  instead of 24h as we want to represent 1
day similar to how we have mentioned for log_rotation_age:
index a2ac7575ca..7284edfbc1 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -337,6 +337,7 @@
 #wal_sender_timeout = 60s      # in milliseconds; 0 disables
 #track_commit_timestamp = off  # collect timestamp of transaction commit
                                # (change requires restart)
+#idle_replication_slot_timeout = 24h   # in minutes; 0 disables

Regards,
Vignesh