Re: Incorrect logic in XLogNeedsFlush()

On Wed, Sep 10, 2025 at 3:18 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Wed, Sep 10, 2025 at 1:59 AM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
>
> > Though, it seems like LocalMinRecoveryPoint must be getting
> > incorrectly set elsewhere, otherwise this would have guarded us from
> > examining the control file:
> >
> > if (XLogRecPtrIsInvalid(LocalMinRecoveryPoint) && InRecovery)
> > updateMinRecoveryPoint = false;
> >
> > /* Quick exit if already known to be updated or cannot be updated */
> > if (record <= LocalMinRecoveryPoint || !updateMinRecoveryPoint)
> > return false;
>
> That's not quite right. Before the end-of-recovery checkpoint, the
> InRecovery flag is already set to false. This means that even if
> LocalMinRecoveryPoint is invalid, it won't matter, and
> updateMinRecoveryPoint will not be set to false. Since
> LocalMinRecoveryPoint is 0, the condition if (record <=
> LocalMinRecoveryPoint) will also fail, causing the process to continue
> and read from the ControlFile.

Ah, right, I got turned around. My original investigation showed me
that the checkpointer incorrectly read from the ControlFile when I
added the XLogNeedsFlush() precisely because InRecovery is false
outside of the startup process.

What I want is for it to be safe and accurate to call XLogNeedsFlush()
in any backend (one that might flush WAL, that is).

- Melanie