| 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
| 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 |