Re: Parallel heap vacuum

On 9/8/25 17:40, Melanie Plageman wrote:
> On Wed, Aug 27, 2025 at 2:30 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>
>> On Tue, Aug 26, 2025 at 8:55 AM Melanie Plageman
>> <melanieplageman(at)gmail(dot)com> wrote:
>>
>>> If you do parallel worker setup below heap_vacuum_rel(), then how are
>>> you supposed to use those workers to do non-heap table vacuuming?
>>
>> IIUC non-heap tables can call parallel_vacuum_init() in its
>> relation_vacuum table AM callback implementation in order to
>> initialize parallel table vacuum, parallel index vacuum, or both.
>
> Yep, it's just that that is more like a library helper function.
> Currently, in master, the flow is:
>
> leader does:
> heap_vacuum_rel()
> dead_items_alloc()
> parallel_vacuum_init()
> passes parallel_vacuum_main() to parallel context
> lazy_scan_heap()
> ...
> parallel_vacuum_process_all_indexes()
> LaunchParallelWorkers()
> ...
> parallel_vacuum_process_[un]safe_indexes()
>
> For the leader, I don't remember where your patch moves
> LaunchParallelWorkers() -- since it would have to move to before the
> first heap vacuuming phase. When I was imagining what made the most
> sense to make parallelism agnostic of heap, it seemed like it would be
> moving LaunchParallelWorkers() above heap_vacuum_rel(). But I
> recognize that is a pretty big change, and I haven't tried it, so I
> don't know all the complications. (sounds like you mention some
> below).
>
> Basically, right now in master, we expose some functions for parallel
> index vacuuming that people can call in their own table implementation
> of table vacuuming, but we don't invoke any of them in table-AM
> agnostic code. That may work fine for just index vacuuming, but now
> that we are trying to make parts of the table vacuuming itself
> extensible, the interface feels more awkward.
>
> I won't push the idea further because I'm not even sure if it works,
> but the thing that felt the most natural to me was pushing parallelism
> above the table AM.
>
> ...

I took a quick look at the patch this week. I don't have a very strong
opinion on the changes to table AM API, and I somewhat agree with this
impression. It's not clear to me why we should be adding callbacks that
are AM-specific (and only ever called from that particular AM) to the
common AM interface.

I keep thinking about how we handle parallelism in index builds. The
index AM API did not get a bunch of new callbacks, it's all handled
within the existing ambuild() callback. Shouldn't we be doing something
like that for relation_vacuum()?

That is, each AM-specific relation_vacuum() would be responsible for
starting / stopping workers and all that. relation_vacuum would need
some flag indicating it's OK to use parallelism, of course. We might add
some shared infrastructure to make that more convenient, of course.

This is roughly how index builds handle parallelism. It's not perfect
(e.g. the plan_create_index_workers can do unexpected stuff for
non-btree indexes).

Interestingly enough, this seems to be the exact opposite of what
Melanie proposed above, i.e. moving the parallelism "above" the table
AM. Which AFAICS means we'd keep the new table AM callbacks, but move
the calls "up" above the AM-specific parts.

That should be possible, and maybe even cleaner. But ISTM it'd require a
much more extensive refactoring of how vacuum works. Haven't tried, ofc.

I also repeated the stress test / benchmark, measuring impact with
different number of indexes, amount of deleted tuples, etc. Attached is
a PDF summarizing the results for each part of the patch series (with
different number of workers). For this tests, the improvements are
significant only with no indexes - then the 0004 patch saves 30-50%. But
as soon as indexes are added, the index cleanup starts to dominate.

It's just an assessment, I'm not saying we shouldn't parallelize the
heap cleanup. I assume there are workloads for which patches will make
much more difference. But, what would such cases look like? If I want to
maximize the impact of this patch series, what should I do?

FWIW the patch needs rebasing, there's a bit of bitrot. It wasn't very
extensive, so I did that (in order to run the tests), attached is the
result as v19.

This also reminds I heard a couple complaints we don't allow parallelism
in autovacuum. We have parallel index vacuum, but it's disabled in
autovacuum (and users really don't want to run manual vacuum).

That question is almost entirely orthogonal to this patch, of course.
I'm not suggesting this patch has (or even should) to do anything about
it. But I wonder if you have any thoughts on that?

I believe the reason why parallelism is disabled in autovacuum is that
we want autovacuum to be a background process, with minimal disruption
to user workload. It probably wouldn't be that hard to allow autovacuum
to do parallel stuff, but it feels similar to adding autovacuum workers.
That's rarely the solution, without increasing the cost limit.

regards

--
Tomas Vondra