agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedBUG #19628: Uninterruptible vacuum during hash index processing
4+ messages / 3 participants
[nested] [flat]
* BUG #19628: Uninterruptible vacuum during hash index processing
@ 2026-08-18 12:02 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: PG Bug reporting form @ 2026-08-18 12:02 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: sk@zsrv.org
The following bug has been logged on the website:
Bug reference: 19628
Logged by: Sergei Kornilov
Email address: sk@zsrv.org
PostgreSQL version: 16.14
Operating system: ubuntu
Description:
Hello!
During the installation of minor update (from 16.14 to 16.15), I discovered
that PostgreSQL was shutting down for a long time (about 6 minutes) because
it was waiting for the autovacuum process to terminate.
I was able to find the cause in the vacuum implementation for hash indexes
(this database has a fairly big hash index):
1) vacuum executes hashbulkdelete ( src/backend/access/hash/hash.c )
2) LockBufferForCleanup in loop_top calls HOLD_INTERRUPTS() under the hood
3) then we execute hashbucketcleanup , which can take a long time. Here, the
loop calls vacuum_delay_point (which calls CHECK_FOR_INTERRUPTS), but it
does nothing because HOLD_INTERRUPTS is still in effect.
4) Only at the end of hashbucketcleanup RESUME_INTERRUPTS is called from
LWLockRelease
I checked with gdb: hashbucketcleanup is actually executed with a non-zero
InterruptHoldoffCount.
Breakpoint 1, hashbucketcleanup (rel=rel@entry=0x7f1cf95ec080,
cur_bucket=cur_bucket@entry=0,
bucket_buf=bucket_buf@entry=11847, bucket_blkno=bucket_blkno@entry=1,
bstrategy=0x55f74ae62ca8, maxbucket=32573,
highmask=32767, lowmask=16383, tuples_removed=0x7fff95dc5a98,
num_index_tuples=0x7fff95dc5a90, split_cleanup=false,
callback=0x55f7395a6fa8 <vac_tid_reaped>, callback_state=0x7f1ce9b6d048)
at hash.c:691
691 {
(gdb) n
700 if (split_cleanup)
(gdb) n
717 vacuum_delay_point();
(gdb) p InterruptHoldoffCount
$1 = 1
I'm not sure how to properly fix hashbucketcleanup. At the very least, an
additional vacuum_delay_point in hashbulkdelete before LockBufferForCleanup
would improve the situation slightly.
regards, Sergei
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #19628: Uninterruptible vacuum during hash index processing
@ 2026-08-22 14:54 mostafa nabil <mostafa.nabil.nafie@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: mostafa nabil @ 2026-08-22 14:54 UTC (permalink / raw)
To: sk@zsrv.org; pgsql-bugs@lists.postgresql.org
Hi Sergei,
Thanks for the report and root cause analysis.
Attached is a patch adding a vacuum_delay_point() call at the top of
hashbulkdelete()'s per-bucket loop, before any buffer lock is taken.
The function currently has no interrupt check outside the one inside
hashbucketcleanup(), which is ineffective for the reason you already
found (InterruptHoldoffCount stays > 0 for the whole bucket once
LockBufferForCleanup() is called). This adds a check at the one spot
where nothing is locked yet, so a pending shutdown or cancel is
noticed at the next bucket boundary instead of only after the whole
index scan finishes.
No automated test: both patched and unpatched code eventually honor
the cancel, so a TAP test would need a hardcoded time threshold,
which risks flaking on slower CI hosts. Verified manually instead
(script attached): on an 8M-row table with ~90% dead tuples, cancelling
a VACUUM during the "vacuuming indexes" phase took ~2.08s on unpatched
master vs ~0.014s with the patch.
Not addressed: a single bucket with a very long overflow chain.
hashbucketcleanup() uses lock chaining (locks the next overflow page
before releasing the current one) to prevent a race with concurrent
scans overtaking a partially vacuumed bucket, per the hash AM README.
So interrupts are never truly clear during one bucket's own cleanup,
and this patch can't help there without changing the locking scheme.
I'd treat that as a separate, riskier follow-up.
Likely a backpatch candidate (real bug in shipped versions), but
I'll leave that call to whoever reviews this.
Regards,
Mostafa
On Sat, Aug 22, 2026 at 5:34 PM PG Bug reporting form <
noreply@postgresql.org> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 19628
> Logged by: Sergei Kornilov
> Email address: sk@zsrv.org
> PostgreSQL version: 16.14
> Operating system: ubuntu
> Description:
>
> Hello!
>
> During the installation of minor update (from 16.14 to 16.15), I discovered
> that PostgreSQL was shutting down for a long time (about 6 minutes) because
> it was waiting for the autovacuum process to terminate.
>
> I was able to find the cause in the vacuum implementation for hash indexes
> (this database has a fairly big hash index):
>
> 1) vacuum executes hashbulkdelete ( src/backend/access/hash/hash.c )
> 2) LockBufferForCleanup in loop_top calls HOLD_INTERRUPTS() under the hood
> 3) then we execute hashbucketcleanup , which can take a long time. Here,
> the
> loop calls vacuum_delay_point (which calls CHECK_FOR_INTERRUPTS), but it
> does nothing because HOLD_INTERRUPTS is still in effect.
> 4) Only at the end of hashbucketcleanup RESUME_INTERRUPTS is called from
> LWLockRelease
>
> I checked with gdb: hashbucketcleanup is actually executed with a non-zero
> InterruptHoldoffCount.
>
> Breakpoint 1, hashbucketcleanup (rel=rel@entry=0x7f1cf95ec080,
> cur_bucket=cur_bucket@entry=0,
> bucket_buf=bucket_buf@entry=11847, bucket_blkno=bucket_blkno@entry=1,
> bstrategy=0x55f74ae62ca8, maxbucket=32573,
> highmask=32767, lowmask=16383, tuples_removed=0x7fff95dc5a98,
> num_index_tuples=0x7fff95dc5a90, split_cleanup=false,
> callback=0x55f7395a6fa8 <vac_tid_reaped>,
> callback_state=0x7f1ce9b6d048)
> at hash.c:691
> 691 {
> (gdb) n
> 700 if (split_cleanup)
> (gdb) n
> 717 vacuum_delay_point();
> (gdb) p InterruptHoldoffCount
> $1 = 1
>
> I'm not sure how to properly fix hashbucketcleanup. At the very least, an
> additional vacuum_delay_point in hashbulkdelete before LockBufferForCleanup
> would improve the situation slightly.
>
> regards, Sergei
>
>
>
>
>
>
>
--
Mostafa Nabil Software Engineer
Attachments:
[text/x-patch] 0001-Check-for-interrupts-between-hash-index-vacuum-bucke.patch (2.1K, ../../CAOwWfmwvEfJ+_-AQdX_+F_ffRBeTxQn_ru1s19ppX_qTeHnWYg@mail.gmail.com/3-0001-Check-for-interrupts-between-hash-index-vacuum-bucke.patch)
download | inline diff:
From 7f994f1ad50a64cf18a69d0c5063a7612b58dec9 Mon Sep 17 00:00:00 2001
From: Mostafa <mostafa.nabil.nafie@gmail.com>
Date: Sat, 22 Aug 2026 17:25:50 +0300
Subject: [PATCH] Check for interrupts between hash index vacuum buckets
hashbulkdelete() acquires a cleanup lock on each bucket's primary
page and holds it for the duration of hashbucketcleanup(), which
walks every page in that bucket's overflow chain. While that lock
is held, interrupts are held off, so the vacuum_delay_point() call
inside hashbucketcleanup()'s per-page loop is ineffective. The
outer per-bucket loop in hashbulkdelete() had no interrupt check at
all, so a pending shutdown or query cancel was not honored until the
entire index had been scanned, which can take minutes on a table
with many or large buckets.
Add a vacuum_delay_point() call at the top of the per-bucket loop,
before any lock is acquired, so a pending interrupt is noticed at
the next bucket boundary instead of only after the whole index scan
completes. Verified with a manual reproduction: cancelling a VACUUM
mid-"vacuuming indexes" phase on a hash index with many buckets went
from ~2.1s to honor the cancel down to ~0.01s with this change.
This does not address the case of a single bucket with a very long
overflow chain, where the checkpoint added here still cannot fire
until that bucket's own cleanup finishes; that would require
changing how hashbucketcleanup()'s lock chaining works, and is left
as a follow-up.
Reported-by: Sergei Kornilov <sk@zsrv.org>
Discussion: https://postgr.es/m/19628-c2b17d358181a1ea@postgresql.org
---
src/backend/access/hash/hash.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index b2e34d2d45e..796a178f3ec 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -562,6 +562,12 @@ bucket_loop:
Page page;
bool split_cleanup = false;
+ /*
+ * Check for interrupts before acquiring the cleanup lock on the next
+ * bucket.
+ */
+ vacuum_delay_point(false);
+
/* Get address of bucket's start page */
bucket_blkno = BUCKET_TO_BLKNO(cachedmetap, cur_bucket);
--
2.43.0
[application/x-shellscript] test_hash_vacuum_cancel.sh (2.2K, ../../CAOwWfmwvEfJ+_-AQdX_+F_ffRBeTxQn_ru1s19ppX_qTeHnWYg@mail.gmail.com/4-test_hash_vacuum_cancel.sh)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #19628: Uninterruptible vacuum during hash index processing
@ 2026-09-03 06:48 Kirill Reshke <reshkekirill@gmail.com>
parent: mostafa nabil <mostafa.nabil.nafie@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Kirill Reshke @ 2026-09-03 06:48 UTC (permalink / raw)
To: mostafa nabil <mostafa.nabil.nafie@gmail.com>; +Cc: sk@zsrv.org; pgsql-bugs@lists.postgresql.org
On Sat, 22 Aug 2026 at 19:54, mostafa nabil
<mostafa.nabil.nafie@gmail.com> wrote:
>
> Hi Sergei,
>
> Thanks for the report and root cause analysis.
>
> Attached is a patch adding a vacuum_delay_point() call at the top of
> hashbulkdelete()'s per-bucket loop, before any buffer lock is taken.
> The function currently has no interrupt check outside the one inside
> hashbucketcleanup(), which is ineffective for the reason you already
> found (InterruptHoldoffCount stays > 0 for the whole bucket once
> LockBufferForCleanup() is called). This adds a check at the one spot
> where nothing is locked yet, so a pending shutdown or cancel is
> noticed at the next bucket boundary instead of only after the whole
> index scan finishes.
>
> No automated test: both patched and unpatched code eventually honor
> the cancel, so a TAP test would need a hardcoded time threshold,
> which risks flaking on slower CI hosts. Verified manually instead
> (script attached): on an 8M-row table with ~90% dead tuples, cancelling
> a VACUUM during the "vacuuming indexes" phase took ~2.08s on unpatched
> master vs ~0.014s with the patch.
>
> Not addressed: a single bucket with a very long overflow chain.
> hashbucketcleanup() uses lock chaining (locks the next overflow page
> before releasing the current one) to prevent a race with concurrent
> scans overtaking a partially vacuumed bucket, per the hash AM README.
> So interrupts are never truly clear during one bucket's own cleanup,
> and this patch can't help there without changing the locking scheme.
> I'd treat that as a separate, riskier follow-up.
>
> Likely a backpatch candidate (real bug in shipped versions), but
> I'll leave that call to whoever reviews this.
>
> Regards,
> Mostafa
>
I think our fix is fine, we also need to remove vacuum_delay_point
from hashbucketcleanup function, since it is ineffective if called was
Interrupt holdoff. This will follow existing coding practice, see also
how GIN vacuum works with buffer lock/vacuum_delay_point
Also, we can actually test this deterministically using injection
points, but I dont think this test is worth cpu cycles in buildfarm.
Too much for this.
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #19628: Uninterruptible vacuum during hash index processing
@ 2026-09-04 09:47 mostafa nabil <mostafa.nabil.nafie@gmail.com>
parent: Kirill Reshke <reshkekirill@gmail.com>
0 siblings, 0 replies; 4+ messages in thread
From: mostafa nabil @ 2026-09-04 09:47 UTC (permalink / raw)
To: Kirill Reshke <reshkekirill@gmail.com>; pgsql-bugs@lists.postgresql.org; sk@zsrv.org
Hi Kirill,
Thanks for the review.
v2 attached, i removed the vacuum_delay_point() call inside
hashbucketcleanup()'s per-page loop as you suggested. Good catch because
it's dead code.
Regards,
Mostafa
On Fri, Sep 4, 2026 at 12:42 PM mostafa nabil <mostafa.nabil.nafie@gmail.com>
wrote:
> Hi Kirill,
>
> Thanks for the review.
>
> v2 attached, i removed the vacuum_delay_point() call inside
> hashbucketcleanup()'s per-page loop as you suggested. Good catch because
> it's dead code.
>
>
> Regards,
> Mostafa
>
> On Thu, Sep 3, 2026 at 9:48 AM Kirill Reshke <reshkekirill@gmail.com>
> wrote:
>
>> On Sat, 22 Aug 2026 at 19:54, mostafa nabil
>> <mostafa.nabil.nafie@gmail.com> wrote:
>> >
>> > Hi Sergei,
>> >
>> > Thanks for the report and root cause analysis.
>> >
>> > Attached is a patch adding a vacuum_delay_point() call at the top of
>> > hashbulkdelete()'s per-bucket loop, before any buffer lock is taken.
>> > The function currently has no interrupt check outside the one inside
>> > hashbucketcleanup(), which is ineffective for the reason you already
>> > found (InterruptHoldoffCount stays > 0 for the whole bucket once
>> > LockBufferForCleanup() is called). This adds a check at the one spot
>> > where nothing is locked yet, so a pending shutdown or cancel is
>> > noticed at the next bucket boundary instead of only after the whole
>> > index scan finishes.
>> >
>> > No automated test: both patched and unpatched code eventually honor
>> > the cancel, so a TAP test would need a hardcoded time threshold,
>> > which risks flaking on slower CI hosts. Verified manually instead
>> > (script attached): on an 8M-row table with ~90% dead tuples, cancelling
>> > a VACUUM during the "vacuuming indexes" phase took ~2.08s on unpatched
>> > master vs ~0.014s with the patch.
>> >
>> > Not addressed: a single bucket with a very long overflow chain.
>> > hashbucketcleanup() uses lock chaining (locks the next overflow page
>> > before releasing the current one) to prevent a race with concurrent
>> > scans overtaking a partially vacuumed bucket, per the hash AM README.
>> > So interrupts are never truly clear during one bucket's own cleanup,
>> > and this patch can't help there without changing the locking scheme.
>> > I'd treat that as a separate, riskier follow-up.
>> >
>> > Likely a backpatch candidate (real bug in shipped versions), but
>> > I'll leave that call to whoever reviews this.
>> >
>> > Regards,
>> > Mostafa
>> >
>>
>> I think our fix is fine, we also need to remove vacuum_delay_point
>> from hashbucketcleanup function, since it is ineffective if called was
>> Interrupt holdoff. This will follow existing coding practice, see also
>> how GIN vacuum works with buffer lock/vacuum_delay_point
>>
>> Also, we can actually test this deterministically using injection
>> points, but I dont think this test is worth cpu cycles in buildfarm.
>> Too much for this.
>>
>> --
>> Best regards,
>> Kirill Reshke
>>
>
>
> --
> Mostafa Nabil Software Engineer
>
--
Mostafa Nabil Software Engineer
Attachments:
[text/x-patch] v2-0001-Check-for-interrupts-between-hash-index-vacuum-bu.patch (2.8K, ../../CAOwWfmy+7Xgu_8O0iWZ8-hnXedhW=0Kv2yqO0yNXVbc6_g3H2A@mail.gmail.com/3-v2-0001-Check-for-interrupts-between-hash-index-vacuum-bu.patch)
download | inline diff:
From 1e69f738bd4bb0499f029f34ee3c9d575b7fe68d Mon Sep 17 00:00:00 2001
From: Mostafa <mostafa.nabil.nafie@gmail.com>
Date: Fri, 4 Sep 2026 12:33:21 +0300
Subject: [PATCH v2] Check for interrupts between hash index vacuum buckets
hashbulkdelete() acquires a cleanup lock on each bucket's primary
page and holds it for the duration of hashbucketcleanup(), which
walks every page in that bucket's overflow chain. While that lock
is held, interrupts are held off, so the vacuum_delay_point() call
inside hashbucketcleanup()'s per-page loop was never effective. The
outer per-bucket loop in hashbulkdelete() had no interrupt check at
all, so a pending shutdown or query cancel was not honored until the
entire index had been scanned, which can take minutes on a table
with many or large buckets.
Add a vacuum_delay_point() call at the top of the per-bucket loop,
before any lock is acquired, so a pending interrupt is noticed at
the next bucket boundary instead of only after the whole index scan
completes. Verified with a manual reproduction: cancelling a VACUUM
mid-"vacuuming indexes" phase on a hash index with many buckets went
from ~2.1s to honor the cancel down to ~0.01s with this change.
Also remove the now-redundant vacuum_delay_point() call inside
hashbucketcleanup()'s per-page loop, since it never had any effect
there: it runs while the bucket's cleanup lock is held, so
InterruptHoldoffCount is always > 0 at that point. This matches how
GIN vacuum places its delay points only where no buffer lock is
held (see ginVacuumPostingTreeLeaves()).
This does not address the case of a single bucket with a very long
overflow chain, where interrupts still cannot be noticed until that
bucket's own cleanup finishes; that would require changing how
hashbucketcleanup()'s lock chaining works, and is left as a
follow-up.
Reported-by: Sergei Kornilov <sk@zsrv.org>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/19628-c2b17d358181a1ea@postgresql.org
---
src/backend/access/hash/hash.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index b2e34d2d45e..7684a2ae5fe 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -562,6 +562,12 @@ bucket_loop:
Page page;
bool split_cleanup = false;
+ /*
+ * Check for interrupts before acquiring the cleanup lock on the next
+ * bucket.
+ */
+ vacuum_delay_point(false);
+
/* Get address of bucket's start page */
bucket_blkno = BUCKET_TO_BLKNO(cachedmetap, cur_bucket);
@@ -799,8 +805,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);
--
2.43.0
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-09-04 09:47 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-18 12:02 BUG #19628: Uninterruptible vacuum during hash index processing PG Bug reporting form <noreply@postgresql.org>
2026-08-22 14:54 ` mostafa nabil <mostafa.nabil.nafie@gmail.com>
2026-09-03 06:48 ` Kirill Reshke <reshkekirill@gmail.com>
2026-09-04 09:47 ` mostafa nabil <mostafa.nabil.nafie@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox