> On 5 Apr 2023, at 20:55, Robert Haas <robertmh...@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