Hi,
On Wed, Sep 23, 2026 at 5:39 PM Masahiko Sawada <
sawada.mshk@gmail.com> wrote:
>
> Good catch, we should fix it.
>
> > The other attached patch adds an optional callback to the worker-finish
> > wait. Parallel autovacuum uses it to handle reloads and poll cost-limit
> > rebalancing every 100 ms; other callers keep the existing behavior. It also
> > adds tests for both cases to
001_parallel_autovacuum.pl.
>
> I've confirmed that the patch fixes the issue. While it works fine,
> I'm a bit concerned that adding
> WaitForParallelWorkersToFinishWithCallback() with a callback and a
> timeout might be overkill, as I don't see any usecase other than
> parallel autovacuum that needs to pass a callback.
>
> An alternative approach would be to have a function in
> vacuumparallel.c that waits for all index statuses to become
> PARALLEL_INDVAC_STATUS_COMPLETED while periodically checking for cost
> parameter updates. We still need the timeout there since nothing wakes
> up the leader on a cost limit rebalance. While it adds another wait
> loop before WaitForParallelWorkersToFinish(), that call should return
> almost immediately. We can consider adding a callback to
> WaitForParallelWorkersToFinish() when we find other use cases in the
> future.
Thanks for reporting the issue. I believe this issue can happen fairly often in practice, especially when the autovacuum worker (leader) gets smaller or fewer indexes than the parallel workers for index vacuuming (workers), and the workers take longer than the leader (for example, non-core indexes that can take a while). So I think we do need to fix it.
A separate wait function would lose all the cases that the existing wait loop for parallel workers, WaitForParallelWorkersToFinish(), already handles, like detecting error messages reported by the workers, their liveness checks, attach/detach, and so on. It would also likely end up duplicating that same logic along with the cost param update code at the end.
I also don't like the wait-100ms-wakeup approach that a separate wait function would run all the time, since it wastes power and CPU cycles. Imagine a worker vacuuming an index that is hundreds of GBs or even TBs, while the leader has only a small index and finishes first. The leader then sits in the wait loop for longer until the large index is done, waking up every 100ms the whole time. So I would prefer not to go that route.
How about doing the check inside the existing wait loop, only when the process is an autovacuum worker, along the lines of the attached WIP? This is simple, the pattern already exists elsewhere in the code, and it looks safe. I also believe the wait loop is not in a performance-critical hot path, and this check is not costly. I checked that it still fixes the reported issues.
What I am less sure about is the fix for the second issue in the attached WIP patch, which needs any leader waiting for its workers to be woken up after the cost limit is rebalanced. I haven't found a better one yet.
PS: I noticed the attached v2 patch posted upthread only after finishing my response.
Thoughts?
--
Bharath Rupireddy