From 7c8e7111f321f3e5f4dd32e865f3612162754981 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 2 Sep 2025 11:32:24 -0400 Subject: [PATCH v2 2/9] Split FlushBuffer() into two parts Before adding write combining to write a batch of blocks when flushing dirty buffers, refactor FlushBuffer() into the preparatory step and actual buffer flushing step. This provides better symmetry with the batch flushing code. --- src/backend/storage/buffer/bufmgr.c | 103 ++++++++++++++++++++-------- 1 file changed, 76 insertions(+), 27 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f3668051574..84ff5e0f1bf 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -529,8 +529,13 @@ static inline BufferDesc *BufferAlloc(SMgrRelation smgr, static bool AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress); static void CheckReadBuffersOperation(ReadBuffersOperation *operation, bool is_complete); static Buffer GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context); +static bool PrepareFlushBuffer(BufferDesc *bufdesc, uint32 *buf_state, XLogRecPtr *lsn); +static void DoFlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, + IOContext io_context, XLogRecPtr buffer_lsn); static void FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, IOContext io_context); +static void CleanVictimBuffer(BufferAccessStrategy strategy, + BufferDesc *bufdesc, uint32 *buf_state, bool from_ring); static void FindAndDropRelationBuffers(RelFileLocator rlocator, ForkNumber forkNum, BlockNumber nForkBlock, @@ -2414,12 +2419,7 @@ GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context) continue; } - /* OK, do the I/O */ - FlushBuffer(buf_hdr, NULL, IOOBJECT_RELATION, io_context); - LWLockRelease(content_lock); - - ScheduleBufferTagForWriteback(&BackendWritebackContext, io_context, - &buf_hdr->tag); + CleanVictimBuffer(strategy, buf_hdr, &buf_state, from_ring); } if (buf_state & BM_VALID) @@ -4246,20 +4246,81 @@ static void FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, IOContext io_context) { - XLogRecPtr recptr; - ErrorContextCallback errcallback; - instr_time io_start; - Block bufBlock; - char *bufToWrite; uint32 buf_state; + XLogRecPtr lsn; + + if (PrepareFlushBuffer(buf, &buf_state, &lsn)) + DoFlushBuffer(buf, reln, io_object, io_context, lsn); +} + +/* + * Prepare to write and write a dirty victim buffer. + */ +static void +CleanVictimBuffer(BufferAccessStrategy strategy, + BufferDesc *bufdesc, uint32 *buf_state, bool from_ring) +{ + + XLogRecPtr max_lsn = InvalidXLogRecPtr; + LWLock *content_lock; + IOContext io_context = IOContextForStrategy(strategy); + + Assert(*buf_state & BM_DIRTY); + + /* Set up this victim buffer to be flushed */ + if (!PrepareFlushBuffer(bufdesc, buf_state, &max_lsn)) + return; + DoFlushBuffer(bufdesc, NULL, IOOBJECT_RELATION, io_context, max_lsn); + content_lock = BufferDescriptorGetContentLock(bufdesc); + LWLockRelease(content_lock); + ScheduleBufferTagForWriteback(&BackendWritebackContext, io_context, + &bufdesc->tag); +} + +/* + * Prepare the buffer with budesc for writing. buf_state and lsn are output + * parameters. Returns true if the buffer acutally needs writing and false + * otherwise. + */ +static bool +PrepareFlushBuffer(BufferDesc *bufdesc, uint32 *buf_state, XLogRecPtr *lsn) +{ /* * Try to start an I/O operation. If StartBufferIO returns false, then * someone else flushed the buffer before we could, so we need not do * anything. */ - if (!StartBufferIO(buf, false, false)) - return; + if (!StartBufferIO(bufdesc, false, false)) + return false; + + *lsn = InvalidXLogRecPtr; + *buf_state = LockBufHdr(bufdesc); + + /* + * Run PageGetLSN while holding header lock, since we don't have the + * buffer locked exclusively in all cases. + */ + if (*buf_state & BM_PERMANENT) + *lsn = BufferGetLSN(bufdesc); + + /* To check if block content changes while flushing. - vadim 01/17/97 */ + *buf_state &= ~BM_JUST_DIRTIED; + UnlockBufHdr(bufdesc, *buf_state); + return true; +} + +/* + * Actually do the write I/O to clean a buffer. + */ +static void +DoFlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, + IOContext io_context, XLogRecPtr buffer_lsn) +{ + ErrorContextCallback errcallback; + instr_time io_start; + Block bufBlock; + char *bufToWrite; /* Setup error traceback support for ereport() */ errcallback.callback = shared_buffer_write_error_callback; @@ -4277,18 +4338,6 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, reln->smgr_rlocator.locator.dbOid, reln->smgr_rlocator.locator.relNumber); - buf_state = LockBufHdr(buf); - - /* - * Run PageGetLSN while holding header lock, since we don't have the - * buffer locked exclusively in all cases. - */ - recptr = BufferGetLSN(buf); - - /* To check if block content changes while flushing. - vadim 01/17/97 */ - buf_state &= ~BM_JUST_DIRTIED; - UnlockBufHdr(buf, buf_state); - /* * Force XLOG flush up to buffer's LSN. This implements the basic WAL * rule that log updates must hit disk before any of the data-file changes @@ -4306,8 +4355,8 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, * disastrous system-wide consequences. To make sure that can't happen, * skip the flush if the buffer isn't permanent. */ - if (buf_state & BM_PERMANENT) - XLogFlush(recptr); + if (!XLogRecPtrIsInvalid(buffer_lsn)) + XLogFlush(buffer_lsn); /* * Now it's safe to write the buffer to disk. Note that no one else should -- 2.43.0