public inbox for [email protected]  

help / color / mirror / Atom feed

From: Sami Imseih <[email protected]>
To: Yugo Nagata <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Pgsql Hackers <[email protected]>
Subject: Re: Track skipped tables during autovacuum and autoanalyze
Date: Fri, 27 Mar 2026 11:48:27 -0500
Message-ID: <CAA5RZ0tNh03LLgJVMA=PXSdY8YVoui4_GyfbfTrYK5cka3Q9Rw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAA5RZ0snnePNW1NKGKh+NyJ1CY26T5F_6-tTq+BHWM2kj1fN1g@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

> I've attached a revised patch reflecting this change, and it also includes
> the documentation.

Thanks fo the update!

I have some comments:

1/
+pgstat_report_skipped_vacuum_analyze(Oid relid, bits8 flags)

using bit8 is fine here, but I would have just used int. For this
case, it's just a matter of prefernace.

2/
+/* flags for pgstat_flush_backend() */
+#define PGSTAT_REPORT_SKIPPED_VACUUM   (1 << 0)        /* vacuum is skipped */
+#define PGSTAT_REPORT_SKIPPED_ANALYZE  (1 << 1)        /* analyze is skipped */
+#define PGSTAT_REPORT_SKIPPED_AUTOVAC  (1 << 2)        /* skipped
during autovacuum/autoanalyze */
+#define PGSTAT_REPORT_SKIPPED_ANY   (PGSTAT_REPORT_SKIPPED_VACUUM |
PGSTAT_REPORT_SKIPPED_ANALYZE)

can we just have 4 flags, SKIPPED_VACUUM, SKIPPED_ANALYZE,
SKIPPED_AUTOVACUUM, SKIPPED_AUTOANALYZE,
which can then remove the nested if/else and makes the mapping more obvious

+       if (flags & PGSTAT_REPORT_SKIPPED_AUTOVAC)
+       {
+               if (flags & PGSTAT_REPORT_SKIPPED_VACUUM)
+               {
+                       tabentry->last_skipped_autovacuum_time = ts;
+                       tabentry->skipped_autovacuum_count++;
+               }
+               if (flags & PGSTAT_REPORT_SKIPPED_ANALYZE)
+               {
+                       tabentry->last_skipped_autoanalyze_time = ts;
+                       tabentry->skipped_autoanalyze_count++;
+               }
+       }
+       else
+       {
+               if (flags & PGSTAT_REPORT_SKIPPED_VACUUM)
+               {
+                       tabentry->last_skipped_vacuum_time = ts;
+                       tabentry->skipped_vacuum_count++;
+               }
+               if (flags & PGSTAT_REPORT_SKIPPED_ANALYZE)
+               {
+                       tabentry->last_skipped_analyze_time = ts;
+                       tabentry->skipped_analyze_count++;
+               }
+       }

3/
For the sake of consistency, can we rename the fields from

skipped_vacuum_count to vacuum_skipped_count, etc. ? to be similar
to fields like vacuum_count

4/
field documentation could be a bit better to match existing phrasing

For example, the timestamp fields:

-       Last time a manual vacuum on this table was attempted but skipped due to
-       lock unavailability (not counting <command>VACUUM FULL</command>)
+       The time of the last manual vacuum on this table that was skipped
+       due to lock unavailability (not counting <command>VACUUM FULL</command>)

and the counter fields

-       Number of times vacuums on this table have been attempted but skipped
+       Number of times a manual vacuum on this table has been skipped

5/
Partitioned table asymmetry between vacuum_count and vacuum_skipped_count.

vacuum_count never increments on a the parenttable, because the parent is never
pocessed. On the other hand, if the manual VACUUM/ANALYZE is on the
parent table,
then we will skip all the children. So, we should still report the skip on the
parent table, but we should add a Notes section in the docs perhaps to
document this caveat?

6/
It would be nice to add a test for this, but this requires concurrency and I'm
not sure it's woth it.

Also, can you create a CF entry in
https://commitfest.postgresql.org/59/, please.

Thanks!

--
Sami Imseih
Amazon Web Services (AWS)

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Track skipped tables during autovacuum and autoanalyze
  In-Reply-To: <CAA5RZ0tNh03LLgJVMA=PXSdY8YVoui4_GyfbfTrYK5cka3Q9Rw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox