| From: | Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> |
|---|---|
| To: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> |
| Cc: | Sami Imseih <samimseih(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Michael Paquier <michael(at)paquier(dot)xyz> |
| Subject: | Re: Track skipped tables during autovacuum and autoanalyze |
| Date: | 2026-08-12 03:00:47 |
| Message-ID: | CAOzEurTDjmozU=fF4W--n6kpKSNuCpAuMn9-FMPMXLOXzj_c5g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Jul 28, 2026 at 11:27 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> Thanks for pointing it out.
> I've updated the patch to fix this.
Thanks. v14 applies cleanly to master and check-world passes for me.
This time I looked at the reporting path itself and have a few
comments.
1. pgstat_report_skipped_vacuum_analyze() accesses the catalog while
holding the stats entry lock.
pgstat_get_entry_ref_locked() returns with the entry's LWLock held,
and the re-check runs under it. A catcache miss there opens pg_class,
so we can wait for a heavyweight lock while holding an LWLock, which
can deadlock. The deadlock detector does not see the LWLock side, and
LWLockAcquire() holds interrupts, so pg_cancel_backend() does not
break the wait either.
Could the re-check simply run after pgstat_unlock_entry()? The same
case then cancels immediately.
2. A skipped TOAST vacuum is also counted as a skipped analyze.
vacuum_rel() copies params for the TOAST recursion and only adds
VACOPT_PROCESS_MAIN, so VACOPT_ANALYZE survives into the recursive
call, and a contended lock on the toast table bumps its
lock_skipped_analyze_count as well. The comment just above that code
says analyze is never done on toast tables, so clearing the flag next
to the VACOPT_PROCESS_MAIN line looks right to me:
```
+ toast_vacuum_params.options &= ~VACOPT_ANALYZE;
```
3. Relations that can never be vacuumed get a stats entry.
The skip is reported before the relkind checks in vacuum_rel() and
analyze_rel(), so VACUUM (SKIP_LOCKED) on a locked view creates a
relation stats entry and bumps the counters. pg_stat_all_tables
filters on relkind, so nothing can ever display them. The function
already reads the pg_class tuple for relisshared, so it could pick up
relkind at the same time and return early for the relkinds those two
functions reject.
4. Smaller things:
- pgstat.h says "/* flags for pgstat_flush_backend() */" above the new
PGSTAT_REPORT_LOCK_SKIPPED_* macros. They are the flags of
pgstat_report_skipped_vacuum_analyze().
- The docs never mention SKIP_LOCKED. A manual command waits for the
lock otherwise, so the four manual columns only advance for commands
using that option. A sentence in the new note would cover it.
- last_lock_skipped_autoanalyze says "by the autovacuum" and "Last
time at which", while the three sibling entries say "by the autovacuum
daemon" and "Last time".
- The four non-auto descrs in pg_proc.dat drop "manual", although the
existing entries next to them say things like 'statistics: last manual
vacuum time for a table'.
Thoughts?
--
Shinya Kato
NTT OSS Center
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hayato Kuroda (Fujitsu) | 2026-08-12 03:06:10 | RE: 019_replslot_limit.pl might fail due to checkpoint skipped |
| Previous Message | Chao Li | 2026-08-12 02:31:45 | Re: [HACKERS] comment/security label for publication/subscription |