From 3b909c1a5b0c1d4f883a25934d3995a6bcdc177e Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 3 Jun 2026 18:08:16 -0400 Subject: [PATCH v15 17/19] Add bgwriter helper BgBufferSync() is long and complex. Put the logic that actually cleans the number of buffers it has decided to clean in its own helper. This is useful for code clarity, but it also allows us to test the bgwriter write combining logic. It's very difficult to directly test any bgwriter functionality reliably. --- src/backend/storage/buffer/bufmgr.c | 150 ++++++++++++++++++---------- src/include/storage/buf_internals.h | 4 + 2 files changed, 102 insertions(+), 52 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 09205bda26f..99c738aa2b1 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4254,6 +4254,97 @@ CheckPointBuffers(int flags) TRACE_POSTGRESQL_BUFFER_SYNC_DONE(NBuffers, num_written, num_to_scan); } +/* + * Clean dirty reusable buffers. + * + * This is the bgwriter's LRU cleaning scan. It works forward from + * next_to_clean and stops after scanning num_to_scan buffers, finding enough + * reusable buffers, or hitting lru_maxpages. BgBufferSync() is long and + * complicated, so this helps simplify it. It also allows us to directly test + * background writer write combining logic. + */ +int +BgBufferSyncCleanBuffers(int lru_maxpages, WritebackContext *wb_context, + int *next_to_clean, uint32 *next_passes, + int *num_to_scan, int *reusable_buffers, + int upcoming_alloc_est, bool *maxwritten_clean) +{ + int num_written = 0; + + /* + * Work on local copies of the in/out parameters to avoid repeated + * dereferencing in the loop. + */ + int to_scan = *num_to_scan; + int clean_idx = *next_to_clean; + uint32 passes = *next_passes; + int reusable = *reusable_buffers; + + *maxwritten_clean = false; + + /* Execute the LRU scan */ + for (; to_scan > 0; to_scan--, clean_idx++) + { + uint64 buf_state; + BufferDesc *bufHdr; + BufferTag tag; + + if (reusable >= upcoming_alloc_est) + break; + + if (clean_idx >= NBuffers) + { + clean_idx = 0; + passes++; + } + + bufHdr = GetBufferDescriptor(clean_idx); + buf_state = pg_atomic_read_u64(&bufHdr->state); + if (BUF_STATE_GET_REFCOUNT(buf_state) != 0 || + BUF_STATE_GET_USAGECOUNT(buf_state) != 0) + continue; + + reusable++; + + /* + * Racy check is fine: bgwriter writes are opportunistic. If we miss a + * buffer that just became dirty, it will be written by a future + * bgwriter pass or by checkpointer. + */ + if (!(buf_state & BM_VALID) || !(buf_state & BM_DIRTY)) + continue; + + /* Make sure we can handle the pin */ + ReservePrivateRefCountEntry(); + ResourceOwnerEnlarge(CurrentResourceOwner); + + if (!PinBuffer(bufHdr, BUC_ZERO, true)) + continue; + + FlushUnlockedBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL); + + /* Snapshot the tag before unpinning */ + tag = bufHdr->tag; + UnpinBuffer(bufHdr); + + ScheduleBufferTagForWriteback(wb_context, IOCONTEXT_NORMAL, &tag); + + if (++num_written >= lru_maxpages) + { + *maxwritten_clean = true; + break; + } + } + + /* Write back the updated scan state for the caller */ + *num_to_scan = to_scan; + *next_to_clean = clean_idx; + *next_passes = passes; + *reusable_buffers = reusable; + + return num_written; +} + /* * BgBufferSync -- Write out some dirty buffers in the pool. * @@ -4304,6 +4395,7 @@ BgBufferSync(WritebackContext *wb_context) int num_to_scan; int num_written; int reusable_buffers; + bool maxwritten_clean; /* Variables for final smoothed_density update */ long new_strategy_delta; @@ -4485,59 +4577,13 @@ BgBufferSync(WritebackContext *wb_context) num_written = 0; reusable_buffers = reusable_buffers_est; - /* Execute the LRU scan */ - for (; num_to_scan > 0; num_to_scan--, next_to_clean++) - { - uint64 buf_state; - BufferDesc *bufHdr; - BufferTag tag; - - if (reusable_buffers >= upcoming_alloc_est) - break; - - if (next_to_clean >= NBuffers) - { - next_to_clean = 0; - next_passes++; - } - - bufHdr = GetBufferDescriptor(next_to_clean); - buf_state = pg_atomic_read_u64(&bufHdr->state); - if (BUF_STATE_GET_REFCOUNT(buf_state) != 0 || - BUF_STATE_GET_USAGECOUNT(buf_state) != 0) - continue; - - reusable_buffers++; - - /* - * Racy check is fine: bgwriter writes are opportunistic. If we miss a - * buffer that just became dirty, it will be written by a future - * bgwriter pass or by checkpointer. - */ - if (!(buf_state & BM_VALID) || !(buf_state & BM_DIRTY)) - continue; - - /* Make sure we can handle the pin */ - ReservePrivateRefCountEntry(); - ResourceOwnerEnlarge(CurrentResourceOwner); - - if (!PinBuffer(bufHdr, BUC_ZERO, true)) - continue; + num_written = BgBufferSyncCleanBuffers(bgwriter_lru_maxpages, wb_context, + &next_to_clean, &next_passes, + &num_to_scan, &reusable_buffers, + upcoming_alloc_est, &maxwritten_clean); - FlushUnlockedBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL); - - /* Snapshot the tag before unpinning */ - tag = bufHdr->tag; - UnpinBuffer(bufHdr); - - ScheduleBufferTagForWriteback(wb_context, IOCONTEXT_NORMAL, &tag); - - if (++num_written >= bgwriter_lru_maxpages) - { - PendingBgWriterStats.maxwritten_clean++; - break; - } - } + if (maxwritten_clean) + PendingBgWriterStats.maxwritten_clean++; PendingBgWriterStats.buf_written_clean += num_written; diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index d107c1c0215..bb752ba4a55 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -589,6 +589,10 @@ extern void TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint64 set_flag extern void EagerCleanBuffer(Buffer bufnum, BufferDesc *buf_hdr, IOContext io_context, WritebackContext *wb_context); +extern int BgBufferSyncCleanBuffers(int lru_maxpages, WritebackContext *wb_context, + int *next_to_clean, uint32 *next_passes, + int *num_to_scan, int *reusable_buffers, + int upcoming_alloc_est, bool *maxwritten_clean); /* freelist.c */ -- 2.43.0