Re: Incorrect logic in XLogNeedsFlush()
Chao Li <li.evan.chao@gmail.com>
From: Chao Li <li.evan.chao@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
Dilip Kumar <dilipbalaut@gmail.com>,
Jeff Davis <pgsql@j-davis.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Andres Freund <andres@anarazel.de>
Date: 2025-09-18T00:11:08Z
Lists: pgsql-hackers
> On Sep 18, 2025, at 07:20, Michael Paquier <michael@paquier.xyz> wrote: > > On Tue, Sep 16, 2025 at 09:40:50AM +0900, Michael Paquier wrote: >> As a whole, the patch looks like a good balance, able to satisfy the >> new case you want to handle, Melanie. I am guessing that you'd want >> to tweak it and apply it yourself, so please feel free. > > Hearing nothing, I'd like to move ahead with this improvement. I have > tweaked a bit the comments, as suggested. If one switches the check > of XLogNeedsFlush() from XLogInsertAllowed() to RecoveryInProgress(), > the recovery test 015 blows up as expected. > > Any opinions or more word-smithing required? > -- > Michael > <v3-0001-Make-XLogFlush-and-XLogNeedsFlush-decision-more-c.patch> My 5 cents: ``` + * XLogInsertAllowed() is used as an end-of-recovery checkpoint is + * launched while recovery is still in progress, RecoveryInProgress() ``` “XLogInsertAllowed() is used as an end-of-recovery checkpoint is launched” sounds like XLogInsertAllowed() is being “used as a checkpoint,” how about rephrase as: XLogInsertAllowed() is needed because an end-of-recovery checkpoint can be launched while recovery is still in progress, Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Commits
-
Reorder XLogNeedsFlush() checks to be more consistent
- bb68cde4136b 19 (unreleased) landed
-
Make XLogFlush() and XLogNeedsFlush() decision-making more consistent
- deb208df4559 19 (unreleased) landed