From 9953c4902c346e8da480fac50e41034ec5b32651 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 2 Jun 2026 18:38:11 -0400 Subject: [PATCH v15 03/19] Make StrategyRejectBuffer encapsulate more logic Before refactoring GetVictimBuffer(), move more of its buffer rejection logic into StrategyRejectBuffer itself. --- src/backend/storage/buffer/bufmgr.c | 9 +-------- src/backend/storage/buffer/freelist.c | 26 ++++++++++++++++++++------ src/include/storage/buf_internals.h | 7 ++++++- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index d1d37595524..f36d1887970 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -71,11 +71,6 @@ #include "utils/timestamp.h" #include "utils/wait_event.h" - -/* Note: these two macros only work on shared buffers, not local ones! */ -#define BufHdrGetBlock(bufHdr) ((Block) (BufferBlocks + ((Size) (bufHdr)->buf_id) * BLCKSZ)) -#define BufferGetLSN(bufHdr) (PageGetLSN(BufHdrGetBlock(bufHdr))) - /* Note: this macro only works on local buffers, not shared ones! */ #define LocalBufHdrGetBlock(bufHdr) \ LocalBufferBlockPointers[-((bufHdr)->buf_id + 2)] @@ -2625,9 +2620,7 @@ again: * inside StrategyGetBuffer(). */ if (strategy && from_ring && - buf_state & BM_PERMANENT && - XLogNeedsFlush(BufferGetLSN(buf_hdr)) && - StrategyRejectBuffer(strategy, buf_hdr, from_ring)) + StrategyRejectBuffer(strategy, buf_hdr, buf_state)) { UnlockReleaseBuffer(buf); goto again; diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c index fdb5bad7910..746740105a9 100644 --- a/src/backend/storage/buffer/freelist.c +++ b/src/backend/storage/buffer/freelist.c @@ -15,6 +15,7 @@ */ #include "postgres.h" +#include "access/xlog.h" #include "pgstat.h" #include "port/atomics.h" #include "storage/buf_internals.h" @@ -741,23 +742,36 @@ IOContextForStrategy(BufferAccessStrategy strategy) * StrategyRejectBuffer -- consider rejecting a dirty buffer * * When a nondefault strategy is used, the buffer manager calls this function - * when it turns out that the buffer selected by StrategyGetBuffer needs to - * be written out and doing so would require flushing WAL too. This gives us - * a chance to choose a different victim. + * when it turns out that the victim buffer selected needs to be written out + * and doing so would require flushing WAL too. This gives us a chance to + * choose a different victim. * * Returns true if buffer manager should ask for a new victim, and false * if this buffer should be written and re-used. */ bool -StrategyRejectBuffer(BufferAccessStrategy strategy, BufferDesc *buf, bool from_ring) +StrategyRejectBuffer(BufferAccessStrategy strategy, BufferDesc *buf, uint64 buf_state) { + Assert(strategy); + /* We only do this in bulkread mode */ if (strategy->btype != BAS_BULKREAD) return false; /* Don't muck with behavior of normal buffer-replacement strategy */ - if (!from_ring || - strategy->buffers[strategy->current] != BufferDescriptorGetBuffer(buf)) + if (strategy->buffers[strategy->current] != BufferDescriptorGetBuffer(buf)) + return false; + + /* + * Rejecting a dirty ring buffer is only useful when reusing it would + * require flushing WAL. This only applies to permanent buffers; unlogged + * buffers can have fake LSNs, so XLogNeedsFlush() is not meaningful for + * them. + */ + if (!(buf_state & BM_PERMANENT)) + return false; + + if (!XLogNeedsFlush(BufferGetLSN(buf))) return false; /* diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 2e0ae8720cc..a527bcd454b 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -146,6 +146,11 @@ StaticAssertDecl(MAX_BACKENDS_BITS <= (BUF_LOCK_BITS - 2), StaticAssertDecl(BM_MAX_USAGE_COUNT < (UINT64CONST(1) << BUF_USAGECOUNT_BITS), "BM_MAX_USAGE_COUNT doesn't fit in BUF_USAGECOUNT_BITS bits"); + +/* Note: these two macros only work on shared buffers, not local ones! */ +#define BufHdrGetBlock(bufHdr) ((Block) (BufferBlocks + ((Size) (bufHdr)->buf_id) * BLCKSZ)) +#define BufferGetLSN(bufHdr) (PageGetLSN(BufHdrGetBlock(bufHdr))) + /* * Buffer tag identifies which disk block the buffer contains. * @@ -580,7 +585,7 @@ extern IOContext IOContextForStrategy(BufferAccessStrategy strategy); extern BufferDesc *StrategyGetBuffer(BufferAccessStrategy strategy, uint64 *buf_state, bool *from_ring); extern bool StrategyRejectBuffer(BufferAccessStrategy strategy, - BufferDesc *buf, bool from_ring); + BufferDesc *buf, uint64 buf_state); extern int StrategySyncStart(uint32 *complete_passes, uint32 *num_buf_alloc); extern void StrategyNotifyBgWriter(int bgwprocno); -- 2.43.0