> On 6 Apr 2023, at 08:39, Masahiko Sawada <sawada.m...@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
