Re: Persist slot invalidations before publishing them

Hi,

On Tue, Sep 29, 2026 at 11:07:34AM +0530, shveta malik wrote:
> >
> Bertrand, I will come to this Assert soon. First I would like to
> think/discuss if we can get rid of passing the 'update_inactive_since'
> boolean altogether. Currently, we need it mainly for two reasons:
>
> a) ReplicationSlotRelease() does not know when it should update
> inactive_since and when it should skip it.
> b) The slot-skip and other invalidation flows currently behave differently.
>
> We could eliminate the second difference by making the logic same for
> both the flows. I don't think there is any harm in skipping the
> 'inactive_since' update for the slot-sync's
> slot-invalidation-persist's error case as well. We never use
> 'inactive_since' to invalidate idle synced slots (see
> CanInvalidateIdleSlot()). And 'inactive_since' only matters for
> synced slots after standby promotion, when it is reset for all synced
> slots by update_synced_slots_inactive_since() from ShutDownSlotSync()
> (promotion's flow). So I don't think we need to maintain separate
> logic for this rare error case. If really needed in the future, we
> could still preserve the current behavior IsSyncingReplicationSlots()
> check in ReplicationSlotRelease(), but I don't think it is worth the
> extra complexity.

Yeah, that makes sense. This is a rare error path, so I agree that it is not
worth the extra complexity.

> That leaves us with just handling the failed-invalidation case where
> ReplicationSlotRelease() need to avoid update of inactive_since. How
> about using a static flag for this? We can set it in the CATCH block
> of ReplicationSlotPersistInvalidation() before calling
> ReplicationSlotRelease().
>
> With this approach, both flows can use ReplicationSlotRelease() in the
> same way and we don't need to split the logic into
> ReplicationSlotReleaseInternal() either. I have attached a sample
> patch. Please let me know your thoughts.

The static flag looks safe, but I wonder if it wouldn't be clearer to keep
ReplicationSlotReleaseInternal() and call it with false from the error path?

That would still allow us to remove update_inactive_since from
ReplicationSlotPersistInvalidation() and its callers, while keeping the
exceptional release behavior explicit.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com