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

shveta malik <shveta.malik@gmail.com>

From: shveta malik <shveta.malik@gmail.com>

To: Amit Kapila <amit.kapila16@gmail.com>

Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, shveta malik <shveta.malik@gmail.com>

Date: 2024-03-25T11:54:25Z

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 →

Add a test for commit ac0e33136a using the injection point.
- 8a695d7998be 18.0 landed
Invalidate inactive replication slots.
- ac0e33136abc 18.0 landed
Fix incorrect slot type in BuildTupleHashTableExt
- d96d1d5152f3 18.0 cited
Allow synced slots to have their inactive_since.
- 6f132ed693b6 17.0 landed
Change last_inactive_time to inactive_since in pg_replication_slots.
- 6d49c8d4b4f4 17.0 landed
Track last_inactive_time in pg_replication_slots.
- a11f330b5584 17.0 landed
Track invalidation_reason in pg_replication_slots.
- 6ae701b4378d 17.0 landed
Add option force_initdb to PostgreSQL::Test::Cluster:init()
- ff9e1e764fcc 17.0 cited
Add a failover option to subscriptions.
- 776621a5e479 17.0 cited
Allow setting failover property in the replication command.
- 73292404370c 17.0 cited
Allow to enable failover property for replication slots via SQL API.
- c393308b69d2 17.0 cited
Track conflict_reason in pg_replication_slots.
- 007693f2a3ac 17.0 cited
Log messages for replication slot acquisition and release.
- 7c3fb505b14e 17.0 cited
Remove vacuum_defer_cleanup_age
- 1118cd37eb61 16.0 cited
Fix corruption due to vacuum_defer_cleanup_age underflowing 64bit xids
- be504a3e974d 16.0 cited
meson: Add initial version of meson based build system
- e6927270cd18 16.0 cited

On Mon, Mar 25, 2024 at 5:10 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> I think we should keep pg_alter_replication_slot() as the last
> priority among the remaining patches for this release. Let's try to
> first finish the primary functionality of inactive_timeout patch.
> Otherwise, I agree that the problem reported by you should be fixed.

Noted. Will focus on v18-002 patch now.

I was debugging the flow and just noticed that RecoveryInProgress()
always returns 'true' during
StartupReplicationSlots()-->RestoreSlotFromDisk() (even on primary) as
'xlogctl->SharedRecoveryState' is always 'RECOVERY_STATE_CRASH' at
that time. The 'xlogctl->SharedRecoveryState' is changed  to
'RECOVERY_STATE_DONE' on primary and to 'RECOVERY_STATE_ARCHIVE' on
standby at a later stage in StartupXLOG() (after we are done loading
slots).

The impact of this is, the condition in RestoreSlotFromDisk() in v20-001:

if (!(RecoveryInProgress() && slot->data.synced))
     slot->last_inactive_time = GetCurrentTimestamp();

is merely equivalent to:

if (!slot->data.synced)
    slot->last_inactive_time = GetCurrentTimestamp();

Thus on primary, after restart, last_inactive_at is set correctly,
while on promoted standby (new primary), last_inactive_at is always
NULL after restart for the synced slots.

thanks
Shveta