Re: BUG #19628: Uninterruptible vacuum during hash index processing

From: Kirill Reshke <reshkekirill(at)gmail(dot)com>
To: mostafa nabil <mostafa(dot)nabil(dot)nafie(at)gmail(dot)com>
Cc: sk(at)zsrv(dot)org, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #19628: Uninterruptible vacuum during hash index processing
Date: 2026-09-03 06:48:10
Message-ID: CALdSSPhMOpUYFFViHSZma=enFkGjjp-T_xoBGGrmL3TNTS+REA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Sat, 22 Aug 2026 at 19:54, mostafa nabil
<mostafa(dot)nabil(dot)nafie(at)gmail(dot)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

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Edwin Polkerman 2026-09-03 11:55:24 Re: BUG #19647: Difference in pg_basebackup behaviour between PostgreSQL <= 16 and >= 17 with pgactive extension
Previous Message PG Bug reporting form 2026-09-03 06:48:00 BUG #19652: to_number() silently truncates over-length integers