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: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Amit Kapila <amit.kapila16@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>, shveta malik <shveta.malik@gmail.com>
Date: 2024-04-05T03:09:13Z
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 Thu, Apr 4, 2024 at 5:53 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.

Thanks for the patch. Tested it , works well. Few cosmetic changes needed:

in 040 test file:
1)
# Capture the inactive_since of the slot from the primary. Note that the slot
# will be inactive since the corresponding subscription is disabled..

2 .. at the end. Replace with one.

2)
# Synced slot on the standby must get its own inactive_since.

. not needed in single line comment (to be consistent with
neighbouring comments)


3)
update_synced_slots_inactive_since():

if (!StandbyMode)
return;

It will be good to add comments here.

thanks
Shveta