Re: Incorrect logic in XLogNeedsFlush()

Dilip Kumar <dilipbalaut@gmail.com>

From: Dilip Kumar <dilipbalaut@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Jeff Davis <pgsql@j-davis.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2025-09-24T11:58:17Z
Lists: pgsql-hackers
On Fri, Sep 19, 2025 at 10:58 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Sep 18, 2025 at 05:07:00PM +0530, Dilip Kumar wrote:
> > I think this comment is a side note which is stating that it is
> > possible that while XLogNeedFlush() is deciding that based on the
> > current flush position or min recovery point parallely someone might
> > flush beyond that point.  And it was existing comment which has been
> > improved by adding min recovery points, so I think it makes sense.
>
> Indeed.  I have kept this one after drinking more caffeine, rewording
> it slightly.
>
> > I tried improving this comment as well. Feel free to disregard it if
> > you think it's not improving it.
>
> The new additions in XLogNeedsFlush() felt overweight, though, so I
> have kept a shorter and reworded version.  Then, applied the result.

Thanks.

> Do we want to make the order of the checks to be more consistent in
> both routines?  These would require a separate set of double-checks
> and review, but while we're looking at this area of the code we may as
> tweak it more..

I see both routines have the same order i.e. first check if
(!XLogInsertAllowed()) and then if (record <= LogwrtResult.Flush),
what am I missing?

-- 
Regards,
Dilip Kumar
Google



Commits

  1. Reorder XLogNeedsFlush() checks to be more consistent

  2. Make XLogFlush() and XLogNeedsFlush() decision-making more consistent