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

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-03-20T05:47:47Z
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 Mon, Mar 18, 2024 at 3:42 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> Looking at 0001:

Thanks for reviewing.

> 1 ===
>
> +       True if this logical slot conflicted with recovery (and so is now
> +       invalidated). When this column is true, check
>
> Worth to add back the physical slot mention "Always NULL for physical slots."?

Will change.

> 2 ===
>
> @@ -1023,9 +1023,10 @@ CREATE VIEW pg_replication_slots AS
>              L.wal_status,
>              L.safe_wal_size,
>              L.two_phase,
> -            L.conflict_reason,
> +            L.conflicting,
>              L.failover,
> -            L.synced
> +            L.synced,
> +            L.invalidation_reason
>
> What about making invalidation_reason close to conflict_reason?

Not required I think. One can pick the required columns in the SELECT
clause anyways.

> 3 ===
>
> - * Maps a conflict reason for a replication slot to
> + * Maps a invalidation reason for a replication slot to
>
> s/a invalidation/an invalidation/?

Will change.

> 4 ===
>
> While at it, shouldn't we also rename "conflict" to say "invalidation_cause" in
> InvalidatePossiblyObsoleteSlot()?

That's inline with our understanding about conflict vs invalidation,
and keeps the function generic. Will change.

> 5 ===
>
> + * rows_removed and wal_level_insufficient are only two reasons
>
> s/are only two/are the only two/?

Will change..

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com