| From: | "Kevin Rocker" <me(at)kevinrocker(dot)com> |
|---|---|
| To: | "Andrey Borodin" <x4mmm(at)yandex-team(dot)ru>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | "Neil Chen" <carpenter(dot)nail(dot)cz(at)gmail(dot)com>, "pgsql-hackers mailing list" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Fix vacuum_delay_point happening inside lock |
| Date: | 2026-08-12 14:46:25 |
| Message-ID: | d6dd6d5d-a257-4c2b-8f82-cff80ea2707e@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Attached is v5, which takes Tom's suggestion for a spin.
0002 adds asserts at the top of vacuum_delay_point(). I picked separate asserts instead of INTERRUPTS_CAN_BE_PROCESSED(), just like Neil.
Running check-world with that assert tripped on hashbucketcleanup().
Since hashbucketcleanup is called assuming a lock is in place, no place in it is safe. Instead, I moved the delay to the per-bucket loop. It's a switch from a per-page delay to a per-bucket delay, but the per-page delay we're removing wasn't valid anyway.
Andrey, let me know if you have concerns about lock chaining/cleanup with this approach. There shouldn't be any issues since we're outside the locked function entirely.
With both patches applied, check-world passes with assertions enabled, and each patch passes on its own.
- Kevin Rocker
| Attachment | Content-Type | Size |
|---|---|---|
| v5-0001-Move-interrupt-checks-out-of-locked-regions.patch | text/x-patch | 4.4 KB |
| v5-0002-Assert-that-vacuum_delay_point-is-called-only-whe.patch | text/x-patch | 1.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-08-12 14:46:52 | Re: [PATCH] Remove redundant ORDER BY from COUNT aggregates |
| Previous Message | Andrey Borodin | 2026-08-12 14:39:10 | Re: [PATCH]Fix pg_xact corruption from subtransaction abort after subcommit |