On Tue, Jul 28, 2026 at 11:27 PM Yugo Nagata <[email protected]> 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
