Re: Track skipped tables during autovacuum and autoanalyze - Mailing list pgsql-hackers

From Yugo Nagata
Subject Re: Track skipped tables during autovacuum and autoanalyze
Date
Msg-id 20260413170551.5ec43ba5a2c848f0d46c6a0b@sraoss.co.jp
Whole thread Raw
In response to Re: Track skipped tables during autovacuum and autoanalyze  (Yugo Nagata <nagata@sraoss.co.jp>)
List pgsql-hackers
Hello Sami Imseih,

On Sat, 28 Mar 2026 16:18:02 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

> On Fri, 27 Mar 2026 11:48:27 -0500
> Sami Imseih <samimseih@gmail.com> wrote:
> 
> > > 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.
> 
> That makes sense, since using int for flags seems common in other
> places in the code. I'm not sure how much it affects performance,
> though.
> 
> > 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
> 
> I am fine with that. In that case, the nested logic would move to the
> caller side.
> 
> > 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
> 
> Hmm, I think skipped_vacuum_count is more consistent with
> fields like last_vacuum and total_vacuum_time, where the modifier
> comes before vacuum/analyze. What do you think about that?
> 
> > 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>)
> 
> I intended to keep consistency with the existing last_vacuum:
> 
>   Last time at which this table was manually vacuumed (not counting VACUUM FULL)
> 
> although "at which" was accidentally omitted. Your suggestion seems
> simpler and more natural to me. Should we prioritize that over consistency?
>  
> > 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
> 
> The "a munual" was also accidentally omitted, so I'll fix it.
> 
> > 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?
> 
> Yeah, we cannot report skips on the children when a manual
> vacuum/analyze on the parent table is skipped. (It might be possible
> to obtain child information with NoLock, but that would not be safe.)
> 
> Therefore, I agree that the best we can do here is to add a note to the
> documentation of last_skipped_vacuum/analyze and skipped_vacuum/analyze_count.
> 
> For example:
> 
>   When a manual vacuum or analyze on a parent table in an inheritance
>   or partitioning hierarchy is skipped, the statistics are recorded
>   only for the parent table, not for its children.
> 
> > 6/
> > It would be nice to add a test for this, but this requires concurrency and I'm
> > not sure it's woth it.
> 
> I'm not sure what meaningful tests we could add for these statistics.
> I couldn't find any existing tests for fields like last_vacuum.

I've attached a patch reflecting your comments on items 1, 2, and 5.
As for items 3, 4, and 6, I am waiting for your comments, so the patch
is left unchanged for now.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

Attachment

pgsql-hackers by date:

Previous
From: Peter Smith
Date:
Subject: Re: Support EXCEPT for ALL SEQUENCES publications
Next
From: Ashutosh Sharma
Date:
Subject: Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication