Add CHECK_FOR_INTERRUPTS in Evict{Rel,All}UnpinnedBuffers

Started by 邱宇航2 months ago3 messages
#1邱宇航
iamqyh@gmail.com
1 attachment(s)

The pg_buffercache_evict_{relation,all} operations can become extremely
slow when dealing with large buffer pools containing many dirty buffers.
This commit adds CHECK_FOR_INTERRUPTS calls within the underlying
Evict{Rel,All}UnpinnedBuffers functions to ensure these operations
cancellable.

And this should be backpatched through v18 where
pg_buffercache_evict_{relation,all} operations are introduced.

Attachments:

0001-Add-CHECK_FOR_INTERRUPTS-in-Evict-Rel-All-UnpinnedBu.patchapplication/octet-stream; name=0001-Add-CHECK_FOR_INTERRUPTS-in-Evict-Rel-All-UnpinnedBu.patch; x-unix-mode=0644Download
From 13c18cda92f396f36c9383dfb062e184d2dbcd0a Mon Sep 17 00:00:00 2001
From: Yuhang Qiu <iamqyh@gmail.com>
Date: Mon, 3 Nov 2025 09:57:02 +0800
Subject: [PATCH] Add CHECK_FOR_INTERRUPTS in Evict{Rel,All}UnpinnedBuffers

The pg_buffercache_evict_{relation,all} operations can become extremely
slow when dealing with large buffer pool containing many dirty buffers.
This commit adds CHECK_FOR_INTERRUPTS calls within the underlying
Evict{Rel,All}UnpinnedBuffers functions to ensure these operations
cancellable.
---
 src/backend/storage/buffer/bufmgr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index e8544acb784..00719c7aea2 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -6685,6 +6685,8 @@ EvictAllUnpinnedBuffers(int32 *buffers_evicted, int32 *buffers_flushed,
 		uint32		buf_state;
 		bool		buffer_flushed;

+		CHECK_FOR_INTERRUPTS();
+
 		buf_state = pg_atomic_read_u32(&desc->state);
 		if (!(buf_state & BM_VALID))
 			continue;
@@ -6735,6 +6737,8 @@ EvictRelUnpinnedBuffers(Relation rel, int32 *buffers_evicted,
 		uint32		buf_state = pg_atomic_read_u32(&(desc->state));
 		bool		buffer_flushed;

+		CHECK_FOR_INTERRUPTS();
+
 		/* An unlocked precheck should be safe and saves some cycles. */
 		if ((buf_state & BM_VALID) == 0 ||
 			!BufTagMatchesRelFileLocator(&desc->tag, &rel->rd_locator))
--
2.43.5

#2Masahiko Sawada
sawada.mshk@gmail.com
In reply to: 邱宇航 (#1)
Re: Add CHECK_FOR_INTERRUPTS in Evict{Rel,All}UnpinnedBuffers

On Sun, Nov 2, 2025 at 6:15 PM 邱宇航 <iamqyh@gmail.com> wrote:

The pg_buffercache_evict_{relation,all} operations can become extremely
slow when dealing with large buffer pools containing many dirty buffers.
This commit adds CHECK_FOR_INTERRUPTS calls within the underlying
Evict{Rel,All}UnpinnedBuffers functions to ensure these operations
cancellable.

And this should be backpatched through v18 where
pg_buffercache_evict_{relation,all} operations are introduced.

Commit eab9e4e27c0c added CFI for pg_buffercache functions such as
pg_buffercache_pages, but it seems not to cover
pg_buffercache_evict_relation() and pg_buffercache_evict_all().
EvictRelUnpinnedBuffers() and EvictAllUnpinnedBuffers() are used only
by pg_buffercache and they are for testing/development use, so it
makes sense to add CFI to these functions as well. I'll push the patch
barring any objections.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

#3Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Masahiko Sawada (#2)
Re: Add CHECK_FOR_INTERRUPTS in Evict{Rel,All}UnpinnedBuffers

On Mon, Nov 3, 2025 at 12:06 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Sun, Nov 2, 2025 at 6:15 PM 邱宇航 <iamqyh@gmail.com> wrote:

The pg_buffercache_evict_{relation,all} operations can become extremely
slow when dealing with large buffer pools containing many dirty buffers.
This commit adds CHECK_FOR_INTERRUPTS calls within the underlying
Evict{Rel,All}UnpinnedBuffers functions to ensure these operations
cancellable.

And this should be backpatched through v18 where
pg_buffercache_evict_{relation,all} operations are introduced.

Commit eab9e4e27c0c added CFI for pg_buffercache functions such as
pg_buffercache_pages, but it seems not to cover
pg_buffercache_evict_relation() and pg_buffercache_evict_all().
EvictRelUnpinnedBuffers() and EvictAllUnpinnedBuffers() are used only
by pg_buffercache and they are for testing/development use, so it
makes sense to add CFI to these functions as well. I'll push the patch
barring any objections.

Pushed (backpatched to v18).

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com