Re: Checkpointer write combining

On Thu, Sep 11, 2025 at 11:33 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> I don’t understand why the two versions are different:
>
> if (XLogNeedsFlush(lsn))
> {
> /*
> * Remove the dirty buffer from the ring; necessary to prevent an
> * infinite loop if all ring members are dirty.
> */
> strategy->buffers[strategy->current] = InvalidBuffer;
> return true;
> }
>
> return false;
>
> VS
>
> if (XLogNeedsFlush(lsn))
> return false;

I think you mean
if (!XLogNeedsFlush(lsn))
{
   return false;
}
// remove buffer
return true

is the same as

if (XLogNeedsFlush(lsn))
{
//remove dirty buffer
return true
}
return false;

Which is true. I've changed it to be like that.

Attached version 7 is rebased and has some bug fixes.

I also added a bonus batch on the end (0007) that refactors
SyncOneBuffer() to use the CAS loop pattern for pinning the buffer
that Andres introduced in 5e89985928795f243. bgwriter is now the only
user of SyncOneBuffer() and it rejects writing out buffers that are
used, so it seemed like a decent use case for this.

- Melanie