Re: Track skipped tables during autovacuum and autoanalyze

> 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)