Re: autovacuum: automatically propagate updated parameters

On Fri, Sep 25, 2026 at 2:41 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Sep 24, 2026 at 4:47 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > > > 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.
> >
> > > It's not great to sprinkle in worker specific code in the generic parallel
> > > handling. My original thinking was to reject the idea, but the vacuum costing
> > > is already used for non-vacuum purposes (and there have been discussions to
> > > rename and generalize it) so with that in mind I am less concerned for this
> > > particular case.
> >
> > Same here. After thinking on the Bharath's patch, I now think it might
> > make sense. I guess it's likely that when autovacuum wants to wait for
> > other workers to finish in other cases like parallel heap vacuum, we
> > would want to use WaitForParallelWorkersToFinish() and want it to
> > update the cost-based delay params during the wait.
> >
> > > > 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.
> >
> > > Not sure I see a better solution either, and we are running short of time
> > > before 19 RC1.
> >
> > While it's a good idea to wake the leader up instead of polling, I'm a
> > bit concerned that we call SetLatch() on all autovacuum workers
> > participating in cost balancing, whereas we need it only in a narrow
> > situation: when the worker is participating in cost balancing, using
> > parallel vacuum, and waiting for its parallel workers to finish.
> > Calling SetLatch() on a process that is not waiting doesn't send a
> > signal, butit still leaves the latch set, causing a spurious wakeup
> > the next time the process waits for something else.
> >
> > I think a condition variable fits better here.
>
> Thanks all for the comments.
>
> I understand the cost of setting the latch and waking up every leader
> participating in rebalancing could be higher. But we only need to wake
> the leaders that are in the wait-for-parallel-workers-to-finish loop.
> Those are the ones that need the rebalanced cost limit, so they can
> propagate it down to their workers still doing index vacuuming. That
> way the workers can adjust to the system needs. The user may have
> changed the cost limits to slow things down or speed them up, and
> either way it is not good for the workers to miss them.
>
> Now, on the cost of setting the latch. It depends on three things.
> First, how many leaders can exist, which depends on
> autovacuum_max_workers and autovacuum_worker_slots. Second, how often
> the number of leaders sharing the cost limit changes, that is, when
> the recount comes out different from the previous count. This happens
> when a leader starts or exits, or when the user sets or removes a
> per-table cost limit, which moves a leader in or out of the sharing
> set the next time that table is vacuumed. Third, how often a leader is
> waiting in the loop at that moment.
>
> Having all three happen together looks fairly rare in practice IMHO.
> And even if it happens often, the cost is small, since set-latch sends
> a signal only to a process that is in a latch wait at that moment. For
> a leader not in the loop, it only marks the latch as set and returns,
> and that leader's next latch wait just returns once early and
> re-checks.

I think the cost argument should be the other way around. Setting the
latch of a leader that is waiting for its parallel workers is exactly
what we want, and using a cv wouldn't change how often that happens.
What I was concerned about is the case where SetLatch() is wasted,
i.e., when the worker is not waiting in that loop. Since the wakeups
we actually need are not frequent, most SetLatch() calls would be
wasted, but a wasted SetLatch() is cheap as you and Mau mentioned. It
only marks the latch set if the worker is not waiting, and at worst
causes one spurious wakeup if the worker is waiting on something else.
And it happens only when the number of workers sharing the cost limit
changes. So I agree that it's acceptable, and given we're close to
RC1, I'm fine with the SetLatch() approach.

> That said, please find the attached v3 patch set. 0001 has only the
> fixes, and 0002 has the tests posted upthread along with the injection
> points. I think 0001 is ready to go in, unless there are further
> comments. I'm fine leaving 0002 here for the record, since I haven't
> reviewed it in depth and I don't think it has had a close review yet,
> and it needs more dev cycles. Anyone with cycles can pick it up for
> HEAD. Given the time left, my suggestion would be to consider 0001 for
> PG19 and HEAD.
>
> Thoughts?

I'll review the 0002 patch in depth and see if it's reasonable to push
some parts of it along with the fix.

As for the 0001 patch, it looks good to me. I have one minor comment,
though it's a matter of personal preference:

+/*
+ * Refresh the cost-based vacuum delay parameters of an autovacuum worker
+ * running a parallel vacuum (leader) and propagate them to its parallel
+ * workers.
+ *
+ * The leader normally does this at its own cost delay points, which it no
+ * longer reaches once it is only waiting for the parallel workers to finish.
+ * It calls this from that wait instead, on every wakeup, to pick up a config
+ * reload and a change in the number of autovacuum workers sharing the cost
+ * limit. Both wake the leader, the second through the latch set by
+ * autovac_recalculate_workers_for_balance() when the count is recalculated.
+ */
+void
+parallel_vacuum_refresh_cost_params(void)

The second paragraph explains how this function is called. Since what
the function does, as well as its name, is not specific to that wait,
I guess it's better to explain it on the caller side, and the comment
newly added in WaitForParallelWorkersToFinish() already explains a
similar thing. Then the function comment can focus on what the
function does and its side effects, such as reloading the
configuration file. For example:

/*
* Reload the configuration file if requested, and refresh the cost-based
* delay parameters of an autovacuum worker running a parallel vacuum
* (leader), propagating any change to its parallel workers.
*/

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com