Re: Incorrect logic in XLogNeedsFlush()

On Fri, Sep 12, 2025 at 08:45:36AM +0530, Dilip Kumar wrote:
> On Fri, Sep 12, 2025 at 8:07 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> +1, it really makes XLogFlush() to directly check using
>> XLogNeedsFlush() after adding the "WAL inserts are allowed" check in
>> XLogNeedsFlush(), this is the best way to avoid any inconsistencies in
>> future as well.
>
> I tried with the attached patch, at least check-world reports no issue.

@@ -2797,7 +2797,7 @@ XLogFlush(XLogRecPtr record)
}

/* Quick exit if already known flushed */
- if (record <= LogwrtResult.Flush)
+ if (!XLogNeedsFlush(record))
return;

Hmm, no. You are making more expensive a check that is written to be
cheap. I was more thinking about an assertion at the bottom of
XLogFlush() once a flush is completed. Input and ideas from others
are of course welcome on the matter.
--
Michael