> 0001: > > - AutoVacuumScores *scores); > + AutoVacuumPriority *priority); > > IMHO we need to minimize these kinds of extraneous changes in this patch > set. AutoVacuumScores still seems accurate enough, even when the struct > contains some extra bool members. > > - * A table whose autovacuum_enabled option is false is > - * automatically skipped (unless we have to vacuum it due to freeze_max_age). > - * Thus autovacuum can be disabled for specific tables. Also, when the > cumulative > - * stats system does not have data about a table, it will be skipped. > + * A table whose autovacuum_enabled option is false is automatically skipped > + * by autovacuum (unless we have to vacuum it due to freeze_max_age), > + * but scores are still computed. Also, when the cumulative stats system > does > + * not have data about a table, threshold-based scores will be zero. > > I don't think we need to update this comment. > > - * One exception to the previous paragraph is for tables nearing wraparound, > - * i.e., those that have surpassed the effective failsafe ages. In that > case, > - * the relfrozen/relminmxid-based score is scaled aggressively so that the > - * table has a decent chance of sorting to the front of the list. > + * Furthermore, for tables nearing wraparound, i.e., those that have > surpassed > + * the effective failsafe ages, the relfrozen/relminmxid-based score is > scaled > + * aggressively so that the table has a decent chance of sorting to the front > + * of the list. > > Or this one.
Fixed both.
> + * Priority scores are always computed. dovacuum and doanalyze are only set
> when
> + * autovacuum is active and enabled for the relation.
>
> I think we should more explicitly state that while scores->needs_vacuum and
> friends are always set regardless of whether autovacuum is enabled, the
> return parameters dovacuum, etc., are not.
I made this more explicit by saying " All fields in AutoVacuumScores
are always computed
regardless of autovacuum settings...." I think that is clear enough.
> Or perhaps we should return
> whether autovacuum is enabled in the struct and consolidate the return
> parameters and the struct members. WDYT?
dovacuum, doanalyze will be unused by callers like the sql function, and putting
them in the struct could make this cleaner, but I don't think it's
worth it to blur
the purpose of the struct. I rather keep it just for score computation purposes.
> 0002:
>
> Seems fine.
I found a bug in my v4 that I fixed.
+ if (elevel > 0 && vac_ins_base_thresh >= 0)
is wrong. It should be:
if (elevel > 0)
{
if (vac_ins_base_thresh >= 0)
> 0004:
>
> + FROM pg_stat_get_autovacuum_priority() S
> + JOIN pg_class C ON C.oid = S.relid
> + LEFT JOIN pg_namespace N ON N.oid = C.relnamespace;
>
> What do you think about ordering by score so this view automatically shows
> the tables most in need of vacuuming/analyzing first?
I thought about that initially, but we don't really have an example where the
data is ordered in a stats view ( or any other catalog view ), and I prefer not
to impose that on the user automatically.
See v5.
--
Sami
v5-0001-Always-compute-autovacuum-priority-scores.patch
Description: Binary data
v5-0002-Add-elevel-parameter-to-relation_needs_vacanalyze.patch
Description: Binary data
v5-0003-Refactor-autovacuum-score-computation-into-comput.patch
Description: Binary data
v5-0004-Add-pg_stat_autovacuum_priority-view.patch
Description: Binary data
