From 2dd15fa9d913c4e643cf70c3b669071c368a858d Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 2 Jun 2026 15:38:47 -0400 Subject: [PATCH v15 02/19] pg_stat_io: Count buffer eviction after invalidation GetVictimBuffer() may double count pg_stat_io evictions/reuses when we attempt to invalidate the buffer but some other backend has already redirtied it or pinned it. This should be rare, but fixing it clarifies the code. --- src/backend/storage/buffer/bufmgr.c | 34 ++++++++++++++++------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 28ce5ff7436..d1d37595524 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2553,6 +2553,7 @@ GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context) Buffer buf; uint64 buf_state; bool from_ring; + bool buf_valid; /* * Ensure, before we pin a victim buffer, that there's a free refcount @@ -2640,8 +2641,21 @@ again: &buf_hdr->tag); } + /* Don't use a stale buf_state value after InvalidateVictimBuffer */ + buf_valid = buf_state & BM_VALID; - if (buf_state & BM_VALID) + /* + * If the buffer has an entry in the buffer mapping table, delete it. This + * can fail because another backend could have pinned or dirtied the + * buffer. + */ + if ((buf_state & BM_TAG_VALID) && !InvalidateVictimBuffer(buf_hdr)) + { + UnpinBuffer(buf_hdr); + goto again; + } + + if (buf_valid) { /* * When a BufferAccessStrategy is in use, blocks evicted from shared @@ -2655,25 +2669,15 @@ again: * counted as IOOP_REUSE in the corresponding strategy context. * * At this point, we can accurately count evictions and reuses, - * because we have successfully claimed the valid buffer. Previously, - * we may have been forced to release the buffer due to concurrent - * pinners or erroring out. + * because we have successfully claimed the valid buffer and + * invalidated its previous tenant. Previously, we may have been + * forced to release the buffer due to concurrent pinners or erroring + * out. */ pgstat_count_io_op(IOOBJECT_RELATION, io_context, from_ring ? IOOP_REUSE : IOOP_EVICT, 1, 0); } - /* - * If the buffer has an entry in the buffer mapping table, delete it. This - * can fail because another backend could have pinned or dirtied the - * buffer. - */ - if ((buf_state & BM_TAG_VALID) && !InvalidateVictimBuffer(buf_hdr)) - { - UnpinBuffer(buf_hdr); - goto again; - } - /* a final set of sanity checks */ #ifdef USE_ASSERT_CHECKING buf_state = pg_atomic_read_u64(&buf_hdr->state); -- 2.43.0