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