Hi all, On Tue, Dec 16, 2025 at 3:20 AM Melanie Plageman <[email protected]> wrote: > > On Mon, Dec 8, 2025 at 2:14 AM Soumya S Murali > <[email protected]> wrote: > > > > Here I made some changes in the logic of the function that only returns a > > real LSN when the buffer actually needs a WAL flush. If the buffer is not > > permanent or doesn’t need WAL, it returns false and set the LSN to > > InvalidXLogRecPtr, by which it can avoid confusions or unsafe downstream > > behaviors. > > Thanks for the suggestions. It would be best if you attached a patch > instead of pasting it in the email like this with no formatting (and > no diff). It is very hard to tell what you've changed. > > - Melanie
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. Regards Soumya
From 6363dd3a6c96a2d7448532ada70631842b7f5d78 Mon Sep 17 00:00:00 2001 From: Soumya S Murali <[email protected]> Date: Wed, 17 Dec 2025 10:01:53 +0530 Subject: [PATCH] fix lock handling and LSN semantics in buffer flush helpers Signed-off-by: Soumya S Murali <[email protected]> --- src/backend/storage/buffer/bufmgr.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 2ea5311aed2..4a1a35a75f5 100644 --- 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; /* Must flush WAL up to this LSN before writing the page */ @@ -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; + } + /* * Now the buffer is a valid flush target. * Switch to exclusive lock for checksum + IO preparation. @@ -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); } /* -- 2.34.1
