Hi,
On 2021-10-30 14:21:01 -0700, Andres Freund wrote:
> Due to bug #17245: [1] I spent a considerably amount of time looking at vacuum
> related code. And I found a few things that I think could stand improvement:
While working on the fix for #17255 (more specifically some cleanup that Peter
suggested in the context), I noticed another thing: Initializing parallelism
as part of dead_items_alloc() is a bad idea. Even if there are comments noting
that oddity.
I don't really see why we should do it this way? There's no "no-parallelism"
path in begin_parallel_vacuum() besides compute_parallel_vacuum_workers(). So
it's not like we might just discover the inability to do parallelism during
parallel initialization?
It's also not particularly helpful to have a begin_parallel_vacuum() that
might not actually begin a parallel vacuum...
Minor nit:
begin_parallel_vacuum()'s comment says:
* On success (when we can launch one or more workers), will set dead_items and
* lps in vacrel for caller.
But it actually doesn't know whether we can start workers. It just checks
max_parallel_maintenance_workers, no?
Greetings,
Andres Freund