MIME-Version: 1.0
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>
In-Reply-To: 
 <CAA5RZ0uiBCiQDUfA5sbc50gybWx1YES3HfLKMd9gv661YpJyRg@mail.gmail.com>
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 31 Mar 2026 11:09:43 -0700
Message-ID: 
 <CALj2ACVnQRS=T8SY2zegfft=Nku1n6w5609JPdd9jqiM+G_gGg@mail.gmail.com>
Subject: Re: Add pg_stat_autovacuum_priority
To: Sami Imseih <samimseih@gmail.com>
Cc: Robert Treat <rob@xzilla.net>, satyanarlapuram@gmail.com,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CALj2ACVnQRS%3DT8SY2zegfft%3DNku1n6w5609JPdd9jqiM%2BG_gGg%40mail.gmail.com>
Precedence: bulk

Hi,

On Mon, Mar 30, 2026 at 11:16=E2=80=AFAM Sami Imseih <samimseih@gmail.com> =
wrote:
>
> > I think we are also in agreement here, although based on my
> > experience, filtering out things like system and toast tables will be
> > common, but I don't see that changing what you said above.  On a
> > similar note, +1 to your changes in v2.
>
> Thanks!
>
> v3 now includes the refactoring [1] suggestion brought up by Alvarro

Thank you for sending the latest patches. Here are some comments:

1/ + while ((classTup =3D heap_getnext(relScan, ForwardScanDirection)) !=3D=
 NULL)

Missing check_for_interrupts call while scanning the pg_class system catalo=
g.

2/
+ avopts =3D 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?

3/
+ values[8] =3D Float8GetDatum(scores.vac_ins);
+ values[9] =3D 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++] =3D Float8GetDatum(scores.anl);
Assert(i =3D=3D NUM_AV_SCORE_COLS);

4/
+     The <link linkend=3D"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?

5/ Also, can we add a simple paragraph on how to interpret and take
actions based on the scores reported (like prioritizing one table over
the other - adjust these parameters in the table's relopts or
something like that - no need to cover all the possible cases, but
just one example would be sufficient for the user to understand)?

6/ +  descr =3D> 'statistics: autovacuum priority scores for all relations'=
,

s/"for all relations"/"for all relations in the current database"

7/ Addition of force_scores to relation_needs_vacanalyze makes the
code unreadable (IMO) with a lot of if-else branching. Why not make
force_vacuum an option and pass it as true from the stats function and
leave a note in the function comment on when to use this parameter?
Would something like that work? Also, when autovacuum is disabled
(either via GUC or via relopts), we don't want to calculate and report
any scores. IMHO, this keeps things simple and code readable.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com