> Thanks for working on this. I read through v9 and I am +1 on the feature. > > It does not build against master for me: > > pgstat_relation.c: In function ‘pgstat_report_skipped_vacuum_analyze’: > pgstat_relation.c:419:17: error: too few arguments to function > ‘pgstat_drop_entry’ > 419 | pgstat_drop_entry(PGSTAT_KIND_RELATION, > | ^~~~~~~~~~~~~~~~~ > In file included from pgstat_relation.c:27: > ../../../../src/include/utils/pgstat_internal.h:811:13: note: declared here > 811 | extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 > objid, > | ^~~~~~~~~~~~~~~~~ > make[4]: *** [<builtin>: pgstat_relation.o] Error 1 > > pgstat_drop_entry() gained a missing_ok argument in commit 850b9218c8e > [1], so the call in pgstat_report_skipped_vacuum_analyze() needs a > fourth argument. Passing true, the way the function-usage path in > pgstat_function.c now does, builds cleanly.
Correct. This is due to 850b9218c8e4. I added 'true' to silently ignore concurrently dropped tables. > On the field names: every skip these fields count is a lock skip, but > skipped_vacuum_count, last_skipped_vacuum and friends do not say so. > > In the "Add autovacuum_warning to surface concurrent vacuum > collisions" thread [2], I raised another skip worth tracking, an > autovacuum worker skipping a table because another worker is already > vacuuming it, as an early sign that autovacuum is not keeping up. > There it was suggested to expose it as a per-table counter in > pg_stat_all_tables, the same surface this patch builds. That needs its > own discussion and I am not proposing to widen this patch for it. > > But since another skip reason may be added later, I think the column > name should make clear the skip is due to lock unavailability. What do > you think? > > [1] https://postgr.es/c/850b9218c8e > [2] > https://www.postgresql.org/message-id/CALj2ACUYYv_z3C5VkcYmyezbqxUuJ%2B0H1GeNGZqPK3WFexmTYw%40mail.gmail.chttps://postgr.es/c/850b9218c8e I am not sure about this. I read [1], and I really don't think that showing the message " table ... is already being vacuumed by another worker" or tracking a counter for this case is needed, but we should keep that discussion in that thread. However, I still agree with your point about making the column name more precise, so doing something like *_lock_skipped_* is better. I don't foresee this, but if we do ever track another skip reason besides locking, it will need to be a separate counter. See v10. [1] https://www.postgresql.org/message-id/flat/CALj2ACUYYv_z3C5VkcYmyezbqxUuJ%2B0H1GeNGZqPK3WFexmTYw%40mail.gmail.com#ccd6fdf16eb76b4e97e6932d24da449c -- Sami Imseih Amazon Web Services (AWS)
v10-0001-Track-skipped-vacuum-and-analyze-activity-per-re.patch
Description: Binary data
