Re: Track skipped tables during autovacuum and autoanalyze

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
Cc: Sami Imseih <samimseih(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Track skipped tables during autovacuum and autoanalyze
Date: 2026-04-13 08:05:51
Message-ID: 20260413170551.5ec43ba5a2c848f0d46c6a0b@sraoss.co.jp
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello Sami Imseih,

On Sat, 28 Mar 2026 16:18:02 +0900
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:

> On Fri, 27 Mar 2026 11:48:27 -0500
> Sami Imseih <samimseih(at)gmail(dot)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(at)sraoss(dot)co(dot)jp>

Attachment Content-Type Size
v4-0001-Track-skipped-vacuum-and-analyze-activity-per-rel.patch text/x-diff 24.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Sharma 2026-04-13 08:10:20 Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication
Previous Message Peter Smith 2026-04-13 07:47:21 Re: Support EXCEPT for ALL SEQUENCES publications