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: Robert Haas <robertmhaas@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Masahiko Sawada <sawada.mshk@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>
Date: 2023-04-05T19:03:53Z
Lists: pgsql-hackers 
> On 5 Apr 2023, at 20:55, Robert Haas <robertmhaas@gmail.com> wrote:

> Again, I don't think this is something we should try to
> address right now under time pressure, but in the future, I think we
> should consider ripping this behavior out.

I would not be opposed to that, but I wholeheartedly agree that it's not the
job of this patch (or any patch at this point in the cycle).

> +               if (autovacuum_vac_cost_limit > 0)
> +                       VacuumCostLimit = autovacuum_vac_cost_limit;
> +               else
> +                       VacuumCostLimit = vacuum_cost_limit;
> +
> +               /* Only balance limit if no cost-related storage
> parameters specified */
> +               if (pg_atomic_unlocked_test_flag(&MyWorkerInfo->wi_dobalance))
> +                       return;
> +               Assert(VacuumCostLimit > 0);
> +
> +               nworkers_for_balance = pg_atomic_read_u32(
> +
> &AutoVacuumShmem->av_nworkersForBalance);
> +
> +               /* There is at least 1 autovac worker (this worker). */
> +               if (nworkers_for_balance <= 0)
> +                       elog(ERROR, "nworkers_for_balance must be > 0");
> +
> +               VacuumCostLimit = Max(VacuumCostLimit /
> nworkers_for_balance, 1);
> 
> I think it would be better stylistically to use a temporary variable
> here and only assign the final value to VacuumCostLimit.

I can agree with that.  Another supertiny nitpick on the above is to not end a
single-line comment with a period.

> Daniel: Are you intending to commit this?

Yes, my plan is to get it in before feature freeze.  I notice now that I had
missed setting myself as committer in the CF to signal this intent, sorry about
that.

--
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