Re: Track skipped tables during autovacuum and autoanalyze

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Yugo Nagata <[email protected]>
To: Yugo Nagata <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Pgsql Hackers <[email protected]>
Subject: Re: Track skipped tables during autovacuum and autoanalyze
Date: Mon, 13 Apr 2026 17:05:51 +0900
Message-ID: <[email protected]> (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]>
	<CAA5RZ0tNh03LLgJVMA=PXSdY8YVoui4_GyfbfTrYK5cka3Q9Rw@mail.gmail.com>
	<[email protected]>

Hello Sami Imseih,

On Sat, 28 Mar 2026 16:18:02 +0900
Yugo Nagata <[email protected]> wrote:

> On Fri, 27 Mar 2026 11:48:27 -0500
> Sami Imseih <[email protected]> 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 <[email protected]>

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: <[email protected]>

* 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