| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
| Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Nikolay Samokhvalov <nik(at)postgres(dot)ai>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: autovacuum: automatically propagate updated parameters |
| Date: | 2026-09-24 23:46:46 |
| Message-ID: | CAD21AoDjAs1TYP=hCbpfeDHu5WziTh_hk6NBzPN2G-k0K1smCw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Thu, Sep 24, 2026 at 2:04 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
> > On 24 Sep 2026, at 08:34, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > On Wed, Sep 23, 2026 at 5:39 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> > 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.
>
> Agree, we should avoid polling loops like that as much as possible.
Agreed.
>
> > 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. The leader can prepare
to sleep on a condition variable in AutoVacuumShmemStruct while
waiting on its latch, like WalSndWait() does, and a process
recalculating the balance can wake it up by broadcasting on it. This
way, we wake up only the leaders that actually need it. That said, I
don't think it's a good idea to make WaitForParallelWorkersToFinish()
prepare to sleep on autovacuum's condition variable. So if we want to
use the condition variable, it seems better to me to have a dedicated
wait function in vacuumparallel.c that waits until all indexes are
completed, and then call WaitForParallelWorkersToFinish().
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-09-25 00:50:02 | Re: autovacuum: automatically propagate updated parameters |
| Previous Message | Michael Paquier | 2026-09-24 23:46:23 | Re: BUG #19715: pg_restore_attribute_stats() rejects range statistics for a domain over int4multirange |