| From: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, John H <johnhyvr(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Introduce XID age based replication slot invalidation |
| Date: | 2026-08-31 14:34:49 |
| Message-ID: | CABdArM4uYTsD2MeD=04EdA4ecty=TKL0Jai5jx0MLBUoXTb1VA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Bharat,
I have started looking into the patches.
On Fri, Aug 28, 2026 at 5:47 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Aug 25, 2026 at 12:58 AM Bertrand Drouvot
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > On Fri, Aug 21, 2026 at 03:05:00PM -0700, Bharath Rupireddy wrote:
> > > Please find the attached v14 patches. 0003 now adds support for
> > > invalidating XID-aged synced replication slots on standbys.
> >
> > Thanks for the new version!
> >
> > I have a few comments:
>
> Thanks for reviewing it and thanks for the off-list conversation.
>
> > === 1
> >
> > +static inline bool
> > +CanInvalidateXidAgedSlot(ReplicationSlot *s)
> > +{
> > + return (max_slot_xid_age != 0 &&
> > + (TransactionIdIsValid(s->data.xmin) ||
> > + TransactionIdIsValid(s->data.catalog_xmin)) &&
> > + !(RecoveryInProgress() && s->data.synced));
> > +}
> >
> > AFAICS this can also invalidate the reserved pg_conflict_detection slot. The
> > checkpoint terminates its logical replication launcher, but the restarted
> > launcher reacquires the invalid slot while subscriptions and workers still
> > consider conflict information retention active.
> >
> > I wonder if invalidating this slot should also transition the affected subscriptions
> > out of conflict information retention, same as stop_conflict_info_retention() is
> > doing?
>
> Since the pg_conflict_detection slot is internally created and
> maintained for the user, I don't think invalidating it (by terminating
> the launcher and asking every apply worker to stop conflict retention)
> is the right approach. After invalidating, the slot cannot be dropped,
> and the conflict detection feature will no longer be working
> correctly, and the launcher will not error out for invalidated slots.
> The user doesn't even have an easy way to deal with this situation.
> They would have already set retain_dead_tuples and
> max_retention_duration for the subscription, but they now have no way
> to get conflict detection to work again.
>
> That said, one of the design goals for the XID-age based slot
> invalidation feature is that no slot blocks vacuum indefinitely.
>
> My proposal is to check the age of each worker's
> oldest_nonremovable_xid against the XID-age GUC in each apply worker
> in should_stop_conflict_info_retention(). If it is aged, stop the
> retention. This mechanism is simple to reason about and fits well with
> the backoff we already have with max_retention_duration. This keeps
> the slot valid but backs off from holding the vacuum.
>
The approach looks good to me. Also with a reasonable
max_retention_duration, retention would normally back off well before
the XID age limit is reached. So, in a well-configured system, this
case may never happen.
My concern is that the launcher freezes the slot's xmin based on
policy, independently of the worker's state. Because of this, a
worker-side check cannot really put a limit on how long the xmin can
be held.
Launcher only advances the slot's xmin when all retention-active
subscriptions are enabled and their workers are running. This is
intentional; otherwise, a disabled subscription could miss conflicts
when it comes back.
This leads to two cases:
1) If a subscription is disabled, or its worker has crashed/cannot be
started, there is no worker to call
should_stop_conflict_info_retention(). Nothing else would release the
old xmin.
2) Even if the worker is running and caught up, its
oldest_nonremovable_xid can be much newer than the slot's frozen
data.xmin. In that case, the worker sees a relatively young XID and
does not back off, while the slot continues to hold the older xmin.
So, if we exempt the slot and rely on the backoff, then in these cases
the slot stays valid, but max_slot_xid_age stops being a bound on how
long
its xmin is held.
One idea would be to drive the check from the launcher rather than the
apply worker. It holds the slot, so it can compare the slot's actual
data.xmin
and can act for subscriptions that have no live worker. A worker-side
check would still be worth having on top of that as a refinement,
letting one lagging subscription back off while the others keep
retaining.
Thoughts?
--
Thanks
Nisha
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2026-08-31 14:47:16 | WAIT FOR NO_THROW option could use some documentation |
| Previous Message | Nathan Bossart | 2026-08-31 14:34:17 | Re: WAIT FOR command should do some query jumbling |