> On 6 Apr 2023, at 08:39, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Also I agree with
> where to put the log but I think the log message should start with
> lower cases:
>
> + elog(DEBUG2,
> + "Autovacuum VacuumUpdateCosts(db=%u, rel=%u,
In principle I agree, but in this case Autovacuum is a name and should IMO in
userfacing messages start with capital A.
> +/*
> + * autovac_recalculate_workers_for_balance
> + * Recalculate the number of workers to consider, given
> cost-related
> + * storage parameters and the current number of active workers.
> + *
> + * Caller must hold the AutovacuumLock in at least shared mode to access
> + * worker->wi_proc.
> + */
>
> Does it make sense to add Assert(LWLockHeldByMe(AutovacuumLock)) at
> the beginning of this function?
That's probably not a bad idea.
> ---
> /* rebalance in case the default cost parameters changed */
> - LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
> - autovac_balance_cost();
> + LWLockAcquire(AutovacuumLock, LW_SHARED);
> + autovac_recalculate_workers_for_balance();
> LWLockRelease(AutovacuumLock);
>
> Do we really need to have the autovacuum launcher recalculate
> av_nworkersForBalance after reloading the config file? Since the cost
> parameters are not used inautovac_recalculate_workers_for_balance()
> the comment also needs to be updated.
If I understand this comment right; there was a discussion upthread that simply
doing it in both launcher and worker simplifies the code with little overhead.
A comment can reflect that choice though.
--
Daniel Gustafsson