Date: Thu, 27 Aug 2026 10:26:49 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org, amit.kapila16@gmail.com
Subject: Re: Persist slot invalidations before publishing them
Message-ID: <apAQ6baJ2etwZt00@bdtpg>
References: <ao7u5I9OeIR72kGp@bdtpg>
 <20260827.170647.1942062682007402092.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20260827.170647.1942062682007402092.horikyota.ntt@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/apAQ6baJ2etwZt00%40bdtpg>
Precedence: bulk

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.pg@gmail.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