That sounds like a good plan to me, backpatch 0001 with the actual fixes and
keep 0002 with the assert on master .
v7 is identical to v6 except with typos fixed.
- Kevin Rocker
From 0e6b96f42d863a556baddbf6fe2eabb3183a6420 Mon Sep 17 00:00:00 2001
From: Kevin Rocker <[email protected]>
Date: Wed, 12 Aug 2026 15:58:04 +0200
Subject: [PATCH v7 1/2] Move interrupt checks out of locked regions
vacuum_delay_point() and CHECK_FOR_INTERRUPTS() cannot process pending
interrupts while interrupts are held. A vacuum delay point may additionally
sleep while retaining a buffer content lock. Several call sites make these
calls while a lock is held.
Move the ANALYZE delay point before scan_analyze_next_block(). Move the
first GIN pending-list cleanup delay point before its locks are acquired;
an existing delay point already covers transitions between pages.
Move the hash bucket cleanup delay point from hashbucketcleanup() up to
hashbulkdelete()'s per-bucket loop, before the bucket's cleanup lock is
acquired. hashbucketcleanup() is called assuming a lock exists for
its duration, so no part of it is a valid call site. Its other callers,
the split-cleanup paths called from insertion, lose the call entirely.
A backend running INSERT doesn't do vacuum cost accounting and there's
an active lock, so the call couldn't sleep there anyway.
dshash sequential iteration returns each stats entry with its partition lock
held and provides no unlocked per-entry boundary, so mark that call with a
grep-friendly comment instead.
Author: Kevin Rocker <[email protected]>
Author: Andrey Borodin <[email protected]>
Reviewed-by: Neil Chen <[email protected]>
Discussion: https://postgr.es/m/492c6247-43d3-477b-8981-fb0c56767b38%40app.fastmail.com
---
src/backend/access/gin/ginfast.c | 5 +++--
src/backend/access/hash/hash.c | 5 +++--
src/backend/commands/analyze.c | 5 ++++-
src/backend/utils/activity/pgstat.c | 4 ++++
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index 46fc60115a8..bb678300b1b 100644
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -797,6 +797,9 @@ ginInsertCleanup(GinState *ginstate, bool must_empty_list,
bool fsm_vac = false;
int workMemory;
+ /* Delay or accept interrupts before acquiring the pending-list locks. */
+ vacuum_delay_point(false);
+
/*
* We would like to prevent concurrent cleanup process. For that we will
* lock metapage in exclusive mode using LockPage() call. Nobody other
@@ -895,8 +898,6 @@ ginInsertCleanup(GinState *ginstate, bool must_empty_list,
*/
processPendingPage(&accum, &datums, page, FirstOffsetNumber);
- vacuum_delay_point(false);
-
/*
* Is it time to flush memory to disk? Flush if we are at the end of
* the pending list, or if we have a full row and memory is getting
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index b2e34d2d45e..3d48355eb08 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -562,6 +562,9 @@ bucket_loop:
Page page;
bool split_cleanup = false;
+ /* Delay or accept interrupts before locking the next bucket. */
+ vacuum_delay_point(false);
+
/* Get address of bucket's start page */
bucket_blkno = BUCKET_TO_BLKNO(cachedmetap, cur_bucket);
@@ -799,8 +802,6 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
bool retain_pin = false;
bool clear_dead_marking = false;
- vacuum_delay_point(false);
-
page = BufferGetPage(buf);
opaque = HashPageGetOpaque(page);
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 15beb8150de..f171527b5a4 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1309,10 +1309,13 @@ acquire_sample_rows(Relation onerel, int elevel,
0);
/* Outer loop over blocks to sample */
- while (table_scan_analyze_next_block(scan, stream))
+ for (;;)
{
vacuum_delay_point(true);
+ if (!table_scan_analyze_next_block(scan, stream))
+ break;
+
while (table_scan_analyze_next_tuple(scan, &liverows, &deadrows, slot))
{
/*
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 4615f610106..51bb78c90be 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1745,6 +1745,10 @@ pgstat_write_statsfile(void)
PgStatShared_Common *shstats;
const PgStat_KindInfo *kind_info = NULL;
+ /*
+ * CHECK_FOR_INTERRUPTS_WITH_INTERRUPTS_HELD: dshash_seq_next()
+ * returns with the current hash partition lock still held.
+ */
CHECK_FOR_INTERRUPTS();
/*
--
2.54.0
From 8e87f4886ff10ad291312639f54e112b696b7d8b Mon Sep 17 00:00:00 2001
From: Kevin Rocker <[email protected]>
Date: Tue, 11 Aug 2026 22:45:12 +0200
Subject: [PATCH v7 2/2] Assert that vacuum_delay_point() is called only when
interruptible.
A delay point may sleep and is expected to service query cancel, so it
must not be reached where CHECK_FOR_INTERRUPTS() cannot act, e.g. with
an LWLock or buffer content lock held. Enforce that in assert-enabled
builds, so that a new call site in a locked region trips the buildfarm
rather than silently delaying with interrupts held off.
Per suggestion from Tom Lane.
Author: Kevin Rocker <[email protected]>
Author: Neil Chen <[email protected]>
Discussion: https://postgr.es/m/492c6247-43d3-477b-8981-fb0c56767b38%40app.fastmail.com
---
src/backend/commands/vacuum.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 64eed16a160..3cde337a494 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -2463,6 +2463,12 @@ vacuum_delay_point(bool is_analyze)
{
double msec = 0;
+ /*
+ * A delay point may sleep and must service query cancel, so it cannot be
+ * reached where CHECK_FOR_INTERRUPTS() would be a no-op.
+ */
+ Assert(INTERRUPTS_CAN_BE_PROCESSED());
+
/* Always check for interrupts */
CHECK_FOR_INTERRUPTS();
--
2.54.0
