| From: | Neil Chen <carpenter(dot)nail(dot)cz(at)gmail(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Cc: | Kevin Rocker <me(at)kevinrocker(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-04 08:13:42 |
| Message-ID: | CAA3qoJnZ3FuNcRzpz13ygzLrvyTqkTOTrahSfQ0iTf=CHMjavw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Andrey,
On Tue, Aug 4, 2026 at 1:45 PM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
> Looks like an interesting and useful improvement.
>
> I've instrumenetd vacuum_delay_point() and CHECK_FOR_INTERRUPTS() with
> __FILE__ and
> __LINE__ and found several other cases when we always check for interrupts
> where
> we cannot actually process them. Let's make a combined patch?
>
> There are couple of cases where I do not know what to do.
>
>
Thanks for the broader audit and the patch.
The GIN and ANALYZE changes look reasonable to me. In particular, the
ANALYZE change restores the ordering that existed before 041b96802ef moved
next_block() into the while condition.
I think the hash case deserves separate discussion. With the current lock
chaining, there is no lock-free per-page boundary. Moving the delay point to
the bucket boundary avoids sleeping while holding a buffer content lock and
should make little difference for short overflow chains. For a long chain,
however, the accumulated delay can be capped at four times
vacuum_cost_delay and then reset, so this may reduce overall throttling
rather than merely making it coarser.
For discussion, I have attached a second, experimental patch on top of v3.
It moves the hash delay point to the bucket boundary and adds these
assertions to vacuum_delay_point():
Assert(InterruptHoldoffCount == 0);
Assert(CritSectionCount == 0);
With assistance from OpenAI Codex (Sol), I tried this locally; it builds and
passes the core regression tests. I have not benchmarked the performance
tradeoff described above.
I am not deeply familiar with the hash AM code, so review from someone more
familiar with this area would be very welcome.
Best regards,
--
Ze Chen (Neil)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0002-Experiment-with-hash-vacuum-delay-point.patch | application/octet-stream | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-08-04 08:13:59 | Re: Checkpoint replication slots later |
| Previous Message | Bharath Rupireddy | 2026-08-04 08:13:00 | Tighten ACL check in repack_is_permitted_for_relation() |