Horton

Re: Should vacuum process config file reload more often

Daniel Gustafsson <daniel@yesql.se>

View thread View on postgr.es
From: Daniel Gustafsson <daniel@yesql.se>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>
Date: 2023-04-06T12:29:34Z
Lists: pgsql-hackers 
> 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

Commits

  1. Fix assertion failure in heap_vacuum_rel

  2. Fix vacuum_cost_delay check for balance calculation.

  3. Fix autovacuum cost debug logging

  4. Refresh cost-based delay params more frequently in autovacuum

  5. Separate vacuum cost variables from GUCs

  6. Make vacuum failsafe_active globally visible

  7. Don't balance vacuum cost delay when per-table settings are in effect