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@gmail.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