> I've been thinking about whether to also return whether autovacuum is
> enabled in the view, i.e., AutoVacuumingActive() && av_enabled.

I don't think we can rely on AutoVacuumingActive() being stable since a
backend that does a SET track_counts = off for whatever reason and
then calls the view will get false. av_enabled will likely be the only
thing we can report.

> So, I'm
> currently leaning towards leaving that information out for now.

I agree.

>
>         scores->needs_vacuum = (vactuples > vacthresh);
>         *do_vacuum |= (av_enabled && scores->needs_vacuum);
>
> ... but others might find your version easier to read.

yeah, for readability, I'll stick with the current.

> Otherwise, 0001 looks good.
>
> In 0003, I think you missed renaming the last argument to
> compute_autovac_score() in table_recheck_autovac().

Earlier, I did not compile with 0003 only. Fixed.

> I didn't see anything else in this read-through.  I'm planning to start
> preparing this for commit tomorrow.

Thanks! here is v7

--
Sami

Attachment: v7-0002-Add-elevel-parameter-to-relation_needs_vacanalyze.patch
Description: Binary data

Attachment: v7-0004-Add-pg_stat_autovacuum_priority-view.patch
Description: Binary data

Attachment: v7-0001-Always-compute-autovacuum-priority-scores.patch
Description: Binary data

Attachment: v7-0003-Refactor-autovacuum-score-computation-into-comput.patch
Description: Binary data

Reply via email to