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

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, shveta malik <shveta.malik@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-04-22T13:50:37Z
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

Hi,

On Thu, Apr 4, 2024 at 9:23 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Thu, Apr 4, 2024 at 4:35 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > > Thanks for the changes. v34-0001 LGTM.
> >
> > I was doing a final review before pushing 0001 and found that
> > 'inactive_since' could be set twice during startup after promotion,
> > once while restoring slots and then via ShutDownSlotSync(). The reason
> > is that ShutDownSlotSync() will be invoked in normal startup on
> > primary though it won't do anything apart from setting inactive_since
> > if we have synced slots. I think you need to check 'StandbyMode' in
> > update_synced_slots_inactive_since() and return if the same is not
> > set. We can't use 'InRecovery' flag as that will be set even during
> > crash recovery.
> >
> > Can you please test this once unless you don't agree with the above theory?
>
> Nice catch. I've verified that update_synced_slots_inactive_since is
> called even for normal server startups/crash recovery. I've added a
> check to exit if the StandbyMode isn't set.
>
> Please find the attached v35 patch.
>

The documentation says about both 'active' and 'inactive_since'
columns of pg_replication_slots say:

---
active bool
True if this slot is currently actively being used

inactive_since timestamptz
The time since the slot has become inactive. NULL if the slot is
currently being used. Note that for slots on the standby that are
being synced from a primary server (whose synced field is true), the
inactive_since indicates the last synchronization (see Section 47.2.3)
time.
---

When reading the description I thought if 'active' is true,
'inactive_since' is NULL, but it doesn't seem to apply for temporary
slots. Since we don't reset the active_pid field of temporary slots
when the release, the 'active' is still true in the view but
'inactive_since' is not NULL. Do you think we need to mention it in
the documentation?

As for the timeout-based slot invalidation feature, we could end up
invalidating the temporary slots even if they are shown as active,
which could confuse users. Do we want to somehow deal with it?

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com