[PATCH v1] Add vacuum_delay_point() to GiST empty-page deletion pass
Hi,
While comparing the vacuum loops of the various index AMs I noticed that
gistvacuum_delete_empty_pages() -- the second pass of a GiST vacuum that
unlinks empty leaf pages -- does not call vacuum_delay_point(), even
though it reads a buffer and issues WAL-logged page deletions for every
internal page it revisits. The other page-scanning loops in
gistvacuum.c, gistvacuumscan() and the recursion in gistvacuumpage(),
both call it.
As a result this phase of a GiST vacuum ignores the cost-based vacuum
delay entirely, and only reacts to query cancellation when a buffer read
happens to perform I/O. On a large GiST index with many emptied leaf
pages this loop can end up walking every internal page of the index.
The attached patch adds a single vacuum_delay_point(false) at the top of
the loop, where no buffer lock is held, matching the sibling idiom. The
function has lacked the call since it was introduced in 7df159a620b.
This is long-standing rather than a recent regression, so I'll leave the
question of back-patching to a committer. Note that on branches before
18 vacuum_delay_point() takes no argument (the bool was added by
e5b0b0ce150), so a back-patch would drop the "false".
Thanks,
Paul Kim
Attachments:
v1-0001-Add-vacuum_delay_point-to-GiST-empty-page-deletio.patchtext/x-patchDownload+3-1
On 21 Jul 2026, at 15:02, Paul Kim <mok03127@gmail.com> wrote:
The attached patch adds a single vacuum_delay_point(false) at the top of
the loop, where no buffer lock is held, matching the sibling idiom. The
function has lacked the call since it was introduced in 7df159a620b.
Yup, this looks like an oversight.
This is long-standing rather than a recent regression, so I'll leave the
question of back-patching to a committer. Note that on branches before
18 vacuum_delay_point() takes no argument (the bool was added by
e5b0b0ce150), so a back-patch would drop the "false".
I don't know whether we should back-patch this. But I think the decision
should be aligned with the similar one in your GIN thread [0]/messages/by-id/178447127453.110.12276981925360691905@mail.gmail.com.
Best regards, Andrey Borodin.
[0]: /messages/by-id/178447127453.110.12276981925360691905@mail.gmail.com
On Tue, Jul 21, 2026 at 05:11:51PM +0500, Andrey Borodin wrote:
On 21 Jul 2026, at 15:02, Paul Kim <mok03127@gmail.com> wrote:
This is long-standing rather than a recent regression, so I'll leave the
question of back-patching to a committer. Note that on branches before
18 vacuum_delay_point() takes no argument (the bool was added by
e5b0b0ce150), so a back-patch would drop the "false".I don't know whether we should back-patch this. But I think the decision
should be aligned with the similar one in your GIN thread [0].
Adding Heikki in CC as the committer of the change that removed
vacuum_delay_point() would attract his attention more easily. Done so
now.
I'd argue that this does not feel like a backpatch is strictly
required here. It took somebody 7 years to notice the gap, and I
suspect that some AI tool has been used for the scanning analysis.
--
Michael
On 2026-07-21, Michael Paquier wrote:
I'd argue that this does not feel like a backpatch is strictly
required here. It took somebody 7 years to notice the gap [...]
Thanks, Michael -- and thanks, Andrey, for confirming the oversight.
Andrey, on aligning this with the GIN thread [0]/messages/by-id/178447127453.110.12276981925360691905@mail.gmail.com: I think GiST is
actually a bit different. The GIN one is a regression -- the call was
there until fd83c83d094 dropped it in a deadlock fix -- so to me it
reads as a back-branch matter, given that on master your proposed
rework would replace that path and carry its own vacuum_delay_point().
This GiST pass, by contrast, has never had the call since 7df159a620b
introduced the two-pass deletion in 2019, and there's no comparable
rework in flight -- so it looks like a standalone master fix rather
than a back-patch.
I don't have a strong view either way, but master-only seems the
natural fit for GiST; I'll leave the final call to you, Heikki, and
Andrey.
[0]: /messages/by-id/178447127453.110.12276981925360691905@mail.gmail.com
--
Paul Kim