Re: Incorrect logic in XLogNeedsFlush()

On Wed, Sep 10, 2025 at 3:18 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Sep 10, 2025 at 1:59 AM Melanie Plageman
> <melanieplageman@gmail.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