Re: Checkpointer write combining

On Tue, Dec 16, 2025 at 11:46 PM Soumya S Murali
<soumyamurali(dot)work(at)gmail(dot)com> wrote:
>
> Thank you for the clarification.
> I understand the concern, and apologies for the earlier confusion. My
> intention is to stay aligned with the direction of your v11 series, so
> this patch contains only small, incremental changes intended to be
> easy to review and not conflict with ongoing work.
> My previous message was only meant to summarize logic I was
> experimenting with locally, not to propose it as a final patch. Based
> on the feedback, I revisited the implementation, made the necessary
> modifications, and verified the updated logic.
> I am attaching the patch for review. It has been tested with make
> check and make -C src/test/isolation check.
> Thank you again for the guidance. I hope this is helpful for the
> ongoing work, and I am looking forward to further feedback.

I took a look at your patch and the first two parts were already fixed
in a previous version

--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4405,7 +4405,7 @@ BufferNeedsWALFlush(BufferDesc *bufdesc, XLogRecPtr *lsn)
UnlockBufHdr(bufdesc);

/* Skip all WAL flush logic if relation is not logged */
- if (!(*lsn != InvalidXLogRecPtr))
+ if (!XLogRecPtrIsValid(*lsn))
return false;

@@ -4433,6 +4433,12 @@ CleanVictimBuffer(BufferDesc *bufdesc, bool
from_ring, IOContext io_context)
content_lock = BufHdrGetContentLock(bufdesc);
LWLockAcquire(content_lock, LW_SHARED);

+ if (!PrepareFlushBuffer(bufdesc, &max_lsn))
+ {
+ LWLockRelease(content_lock);
+ return;
+ }
+

And I don't understand the last parts of the diff.

@@ -4440,19 +4446,14 @@ CleanVictimBuffer(BufferDesc *bufdesc, bool
from_ring, IOContext io_context)
LWLockRelease(content_lock);
LWLockAcquire(content_lock, LW_EXCLUSIVE);

- /*
- * Mark the buffer ready for checksum and write.
- */
- PrepareBufferForCheckpoint(bufdesc, &max_lsn);
+ if (!PrepareFlushBuffer(bufdesc, &max_lsn))
+ {
+ LWLockRelease(content_lock);
+ return;
+ }

/* Release exclusive lock; the batch will write the page later */
LWLockRelease(content_lock);
-
- /*
- * Add LSN to caller's batch tracking.
- * Caller handles XLogFlush() using highest LSN.
- */
- PrepareBufferForCheckpoint(bufdesc, max_lsn);
}

AFAIK, there has never been a function called
PrepareBufferForCheckpoint() and my current patchset doesn't have it
either.

- Melanie