> 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