On Tue, Sep 29, 2026 at 07:10:38AM +0000, Hayato Kuroda (Fujitsu) wrote:
> I like the part you added Assert() in StartupDecodingContext(). This can be
> worked on separately: can anyone which updates ProcGlobal->statusFlags have
> the same Assert()?
I am not sure that we need that, TBH. This feels like an unnecessary
belt-and-suspenders set of assertions.
> Regarding the code, the code comment in ReplicationSlotRelease() may be too detail.
> Can we have something like below? Or adding the possibility that auxiliary processes
> can reach here.
Proposed code and tests have been AI-generated, hence the prose.
Let's simplify that.
> /* avoid unnecessary dirtying shared cache lines */
>
> Regarding the test, I only used to reproduce the issue but not reviewed well,
> because not sure it's aimed to be included. It may need more polish, i.e.,
> advance_wal() has already been defined.
Regarding this part, I am unconvinced that this is worth the cycles
spent on. I am OK to be proved wrong, but for one the test assumes
that we could crash, which is an anti-pattern with the fix in place
because we don't crash once the status flags are not correctly
filtered.
Saying all that, only doing v2-0001 for the slot release seems good
enough here, down to v14. I'd suspect that for some code out there
clearing the flags where we should not is a trap in disguise..
Will process. :)
--
Michael
