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 > <[email protected]> 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
