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

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
Date: 2025-01-21 02:55:39
Message-ID: CAHut+PuZq7ShuMebrO=r3WrXqG6k+1pEGpebgDVdzCDksNGErQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Some review comments for patch v61-0002

======
src/backend/replication/slot.c

1.
* Whether a slot needs to be invalidated depends on the cause. A slot is
- * removed if it:
+ * invalidated if it:
* - RS_INVAL_WAL_REMOVED: requires a LSN older than the given segment
* - RS_INVAL_HORIZON: requires a snapshot <= the given horizon in the given
* db; dboid may be InvalidOid for shared relations
* - RS_INVAL_WAL_LEVEL: is logical
+ * - RS_INVAL_IDLE_TIMEOUT: idle slot timeout has occurred

1a.
Firstly the wording seems odd. "Is invalidated if it:" (missing words?)

~

1b.
Secondly, is this comment strictly correct? IIUC it's not *always*
going to be invalidated just because the cause is one of those listed.
e.g. the code calls InvalidatePossiblyObsoleteSlot but it might not
end up invalidating the slot having a cause RS_INVAL_IDLE_TIMEOUT.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-01-21 02:56:12 Re: per backend I/O statistics
Previous Message Jeff Davis 2025-01-21 02:55:05 Re: Statistics Import and Export