From: Nathan Bossart <nathandbossart@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>
Cc: Robert Treat <rob@xzilla.net>
Cc: satyanarlapuram@gmail.com, pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Add pg_stat_autovacuum_priority
Date: Tue, 31 Mar 2026 13:34:46 -0500
Message-ID: <acwTxpz3Toxt0ty8@nathan> (raw)
In-Reply-To: <CALj2ACVnQRS=T8SY2zegfft=Nku1n6w5609JPdd9jqiM+G_gGg@mail.gmail.com>
References: <CAA5RZ0s4xjMrB-VAnLccC7kY8d0-4806-Lsac-czJsdA1LXtAw@mail.gmail.com>
<CALj2ACXcmYsoJXTYtuGhkxNVXV5jtTNfL-vB+sQq-jE9__N5AA@mail.gmail.com>
<CAA5RZ0surzz41exF5QwecuFU8NqZVRR5aDnC6MObeEcsXhfu4Q@mail.gmail.com>
<CALj2ACX+SRgv2RO9Oo4Me-zzMMjHVg8rf-MqvMRbp9=1ioWbsg@mail.gmail.com>
<CABV9wwM3nRitsgUxeCF0ywbAaLZV5jWC-9tj6WKxkdmQkHRcWg@mail.gmail.com>
<CAA5RZ0vMp2B3UBUoqLVedy8G3u8_O8M11+Y5V7uZv3++CGYasg@mail.gmail.com>
<CABV9wwNBifXpOjxO9rGn1HHK=DG02qApVurWSHa+rDzPriK6pA@mail.gmail.com>
<CAA5RZ0uiBCiQDUfA5sbc50gybWx1YES3HfLKMd9gv661YpJyRg@mail.gmail.com>
<CALj2ACVnQRS=T8SY2zegfft=Nku1n6w5609JPdd9jqiM+G_gGg@mail.gmail.com>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, bharath.rupireddyforpostgres@gmail.com, samimseih@gmail.com, rob@xzilla.net
Subject: Re: Add pg_stat_autovacuum_priority
In-Reply-To: <acwTxpz3Toxt0ty8@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox