On Tue, Mar 31, 2026 at 11:09:43AM -0700, Bharath Rupireddy wrote:
> 1/ + while ((classTup = heap_getnext(relScan, ForwardScanDirection)) != NULL)
>
> Missing check_for_interrupts call while scanning the pg_class system catalog.
From a glance I don't see one in the scanning code in do_autovacuum(),
either. I'm not sure we need to be worried about this.
> + avopts = extract_autovac_opts(classTup, pg_class_desc);
> +
> + compute_autovac_score(classTup, pg_class_desc,
> + effective_multixact_freeze_max_age, avopts,
> + true, &dovacuum, &doanalyze,
> + &wraparound, &scores);
> +
> + if (avopts)
> + pfree(avopts);
>
> When a database has a large number of tables (which is quite common in
> production scenarios), I expect the costs of palloc and pfree being
> used for fetching autovacuum relopts would make this function slower.
> Can we invent a new function or pass a caller-allocated AutoVacOpts
> memory to just copy the relopts and use that in this tight loop when
> scanning for all the relations?
Before making this code more complicated, I think we ought to demonstrate
there's an actual problem or slowness. I am quite dubious we need to do
anything here.
> + values[8] = Float8GetDatum(scores.vac_ins);
> + values[9] = Float8GetDatum(scores.anl);
>
> Nit: It's a matter of taste. How about using something like below
> instead of hardcoded column numbers? I expect this view to grow in the
> future, so it helps to keep things simple.
>
> values[i++] = Float8GetDatum(scores.anl);
> Assert(i == NUM_AV_SCORE_COLS);
I don't think either way is substantially better.
> + The <link linkend="monitoring-pg-stat-autovacuum-priority-view">
> + <structname>pg_stat_autovacuum_priority</structname></link> view can be
> + used to inspect each table's autovacuum need and priority score.
>
> How about adding "as of the moment" to convey that it doesn't report
> what currently running autovacuum or pending autovacuum would
> consider?
I don't think "as of the moment" adds any clarity about that. If we did
want to add something along those lines, I'd add a separate sentence that
says that it doesn't report the values of current autovacuum workers and is
freshly calculated by the current backend (or something like that).
--
nathan
