Re: Persist slot invalidations before publishing them

Thanks for the detailed discussion.

I'm following the reasoning around the durability requirement, particularly
the hot standby case and invalidation causes such as 'inactive_since'.

The separate invalidation file approach makes sense to me as an
alternative, while I also see the advantage of reusing the existing slot
state for backpatching.

I'll hold off on further changes for now and follow the discussion on which
approach is preferred for master implementation.

Regards,
Osama Abdul Qader

On Thu, Aug 27, 2026 at 3:57 PM Bertrand Drouvot <
bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:

> Hi Horiguchi-san,
>
> On Thu, Aug 27, 2026 at 05:06:47PM +0900, Kyotaro Horiguchi wrote:
> > Hello,
> >
> > At Wed, 26 Aug 2026 13:49:24 +0000, Bertrand Drouvot <
> bertranddrouvot(dot)pg(at)gmail(dot)com> wrote in
> > > If ReplicationSlotSave() errors before replacing the state file, the
> slot is
> > > invalid in shared memory but still valid on disk. That sounds
> problematic as the
> > > resource horizon computations could stop accounting for the slot,
> remove required
> > > WAL or rows, and then an immediate restart would restore the old valid
> slot image.
> >
> > I've spent some time looking through the related discussions and
> > patches,
>
> Thanks for looking at it!
>
> > For the InvalidatePossiblyObsoleteSlot() case, at least for
> > RS_INVAL_XID_AGE, if the server crashes after the slot is invalidated
> > but before the invalidation is persisted, it seems that the restored
> > slot would still satisfy the same XID-age condition and would
> > eventually be invalidated again by vacuum or checkpoint. Is the main
> > reason for making the invalidation durable here that we don't want to
> > leave the slot valid until that next opportunity?
>
> Yeah, for XID age the condition should still hold after restart. On a
> primary,
> the end of recovery checkpoint should detect it before connections are
> accepted.
>
> On a hot standby, however, connections can be accepted before the next
> successful
> restartpoint, so a restored slot could be used while valid although rows it
> needed may already have been removed.
>
> Also, other causes are not necessarily rediscovered immediately. For
> example,
> inactive_since is reset at startup, so an idle timeout invalidation would
> not
> be detected again until the timeout has elapsed again.
>
> > If so, I'm a little uncomfortable with persisting a modified copy of
> > the normal slot state before that state has actually been published in
> > shared memory. It seems to make the state transition somewhat harder
> > to follow, since the slot state file no longer necessarily represents
> > the current slot state.
> >
> > Would it be simpler to persist the invalidation separately?
>
> > example, we could write the invalidation cause to a small file such as
> > pg_replslot/<slotname>/invalidated and make it durable before
> > publishing the invalidation in shared memory. On restart, that file
> > would cause the slot to be restored as invalidated with the recorded
> > cause. This would keep the normal slot state file as a representation
> > of the actual slot state, and would also naturally avoid the race with
> > concurrent slot saves.
>
> Your proposal could probably work too. I’m not sure it would be simpler
> though,
> as it would add another on disk state and startup handling. I also could
> not find
> a precedent for introducing such a persistent file in back branches, but I
> may
> have missed one. The proposed patch reuses the existing slot state and
> format,
> which probably makes it more suitable for backpatching.
>
> Regards,
>
> --
> Bertrand Drouvot
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com
>
>
>