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: Amit Kapila <amit.kapila16@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-03-21T17:51:03Z
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 Thu, Mar 21, 2024 at 4:25 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> This makes sense to me. Apart from this, few more comments on 0001.

Thanks for looking into it.

> 1.
> - "%s as caught_up, conflict_reason IS NOT NULL as invalid "
> + "%s as caught_up, invalidation_reason IS NOT NULL as invalid "
>   live_check ? "FALSE" :
> - "(CASE WHEN conflict_reason IS NOT NULL THEN FALSE "
> + "(CASE WHEN conflicting THEN FALSE "
>
> I think here at both places we need to change 'conflict_reason' to
> 'conflicting'.

Basically, the idea there is to not live_check for invalidated logical
slots. It has nothing to do with conflicting. Up until now,
conflict_reason is also reporting wal_removed (although wrongly
including rows_removed, wal_level_insufficient, the two reasons for
conflicts). So, I think invalidation_reason is right for invalid
column. Also, I think we need to change conflicting to
invalidation_reason for live_check. So, I've changed that to use
invalidation_reason for both columns.

> 2.
>
> Can the reasons 'rows_removed' and 'wal_level_insufficient' appear for
> physical slots?

No. They can only occur for logical slots, check
InvalidatePossiblyObsoleteSlot, only the logical slots get
invalidated.

> If not, then it is not clear from above text.

I've stated that "It is set only for logical slots." for rows_removed
and wal_level_insufficient. Other reasons can occur for both slots.

> 3.
> -# Verify slots are reported as non conflicting in pg_replication_slots
> +# Verify slots are reported as valid in pg_replication_slots
>  is( $node_standby->safe_psql(
>   'postgres',
>   q[select bool_or(conflicting) from
> -   (select conflict_reason is not NULL as conflicting
> -    from pg_replication_slots WHERE slot_type = 'logical')]),
> +   (select conflicting from pg_replication_slots
> + where slot_type = 'logical')]),
>   'f',
> - 'Logical slots are reported as non conflicting');
> + 'Logical slots are reported as valid');
>
> I don't think we need to change the comment or success message in this test.

Yes. There the intention of the test case is to verify logical slots
are reported as non conflicting. So, I changed them.

Please find the v14-0001 patch for now. I'll post the other patches soon.

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