Horton

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

Nisha Moond <nisha.moond412@gmail.com>

View thread
From: Nisha Moond <nisha.moond412@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Shlok Kyal <shlok.kyal.oss@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, vignesh C <vignesh21@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-22T05:16:45Z
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

Attachments

On Tue, Jan 21, 2025 at 8:26 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> 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.
>

I feel the phrase "A slot is invalidated if it:" is supposed to be
read alongside the respective cause description, such as:
  "A slot is invalidated if it requires an LSN older than…"
  "A slot is invalidated if it requires a snapshot <= the…"
  "A slot is invalidated if it is logical"

IIUC, each listed cause specifies a clear condition under which the
slot should *always* be invalidated for that cause. To maintain
consistency with the header line "A slot is invalidated if it:", I’ve
modified the description/condition for RS_INVAL_IDLE_TIMEOUT
accordingly. Also, corrected the RS_INVAL_WAL_LEVEL description.

Attached the v62 patch set with above mentioned changes and also
addressed comments at [1].

[1] https://www.postgresql.org/message-id/CAHut%2BPvC4uPabeGMvDuTQ4S%2B5eX66Y6%2BtU5QMRmB2jDw-Cj2Cw%40mail.gmail.com

--
Thanks,
Nisha