From 62ec349f4a0acabe269e718222a4288fa29272cf Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 3 Jun 2026 18:08:16 -0400 Subject: [PATCH v16 17/21] Add bgwriter helper to write buffers 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 upcoming 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 cb5b60c5129..2391fecae12 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4138,6 +4138,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 +BgwriterWriteBuffers(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. * @@ -4188,6 +4279,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; @@ -4369,59 +4461,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 = BgwriterWriteBuffers(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 24c19a71220..32853da256a 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -600,6 +600,10 @@ extern void TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint64 set_flag extern void WriteBufferAndNeighbors(Buffer bufnum, BufferDesc *buf_hdr, IOContext io_context, WritebackContext *wb_context); +extern int BgwriterWriteBuffers(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.47.3