Re: Vacuum statistics

Alena Rybakina <lena.ribackina@yandex.ru>

From: Alena Rybakina <lena.ribackina@yandex.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Masahiko Sawada <sawada.mshk@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Jim Nasby <jnasby@upgrade.com>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Kirill Reshke <reshkekirill@gmail.com>, Melanie Plageman <melanieplageman@gmail.com>, jian he <jian.universality@gmail.com>, Sami Imseih <samimseih@gmail.com>, vignesh C <vignesh21@gmail.com>, Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>, Andrey Borodin <x4mmm@yandex-team.ru>, Andrei Zubkov <zubkov@moonset.ru>, Andrei Lepikhov <lepihov@gmail.com>
Date: 2026-06-18T03:32:27Z
Lists: pgsql-hackers

Attachments

Hi, Alexander! Thank you for the careful review. All five points are 
addressed in the attached,
reworked patch set (now split into 8 per-category patches).

On 16.06.2026 20:30, Alexander Korotkov wrote:
> Some notes about it.
> 1) PgStat_CommonCounts.interrupts_count is never incremented.
Fixed. It's now incremented from vacuum_error_callback() (when 
geterrlevel() == ERROR and a relation is set) via the new 
pgstat_report_vacuum_error(), which bumps interrupts_count in the 
PGSTAT_KIND_VACUUM_DB shmem entry. It writes
directly to shared memory because the vacuum's transaction is aborting, 
and is reported at the database level
(pg_stat_vacuum_database). New TAP test 015_vacuum_stats_interrupts.pl 
cancels a running VACUUM and checks
the counter advances.

(patch 0004)
> 2) LVRelState.wraparound_failsafe_count can only be incremented once
> in lazy_check_wraparound_failsafe().  Should we replace it with a
> bool?
Done. LVRelState.wraparound_failsafe is now a bool, set to true in
lazy_check_wraparound_failsafe(). The per-relation view reports it as a 
0/1 flag reflecting
the latest vacuum, and the per-database aggregate sums those flags into 
a count. This metric
is covered by 005_vacuum_stats_failsafe.pl (xid_wraparound module), 
which burns enough
XIDs to engage the failsafe and asserts the counters advance.
(patch 0004)
> 3) Vacuum statistics isn't counted correctly for parallel index
> vacuum.  parallel_vacuum_process_one_index() doesn't call neither
> extvac_stats_start_idx(), extvac_stats_end_idx(), or
> extvac_accumulate_idx_repor().  This leads all index vacuum stats to
> go the heap stats.  I also think you need a reliable test cases to
> cover issues like this.
Fixed. parallel_vacuum_process_one_index() now samples each index pass with
extvac_stats_start_idx() / extvac_stats_end_idx() and accumulates it via
extvac_accumulate_idx_report() into the index's DSM-resident totals. The 
leader reports those
totals once per index in parallel_vacuum_end() and feeds them back 
through idx_heap_total, so
the per-index buffer/WAL/timing usage is subtracted from the parent heap 
instead of being charged to it.

For the reliable test, I added a TAP test 
(src/test/modules/test_misc/t/016_vacuum_stats_parallel.pl).
It forces the parallel path and verifies it was actually taken by 
checking the VACUUM (VERBOSE) output for a launched
parallel worker. It then vacuums two identical tables - one with 
PARALLEL 2, one with PARALLEL 0 - and asserts that
the per-index statistics (tuples_deleted, pages_deleted, wal_records) 
are identical between the two paths,
which is exactly the invariant that was broken before: the parallel path 
now neither loses the index stats nor
charges them to the heap.
(machinery + parallel sampling and the parallel test in patch 0003, "WAL 
metrics and the sampling machinery")
> 4) It seems that pgstat_vacuum_db_flush_cb() and
> pgstat_vacuum_relation_flush_cb() are unused as
> pgstat_report_vacuum_extstats() reports directly to shmem.  What was
> your intention here?

Good catch! Fixed! pgstat_report_vacuum_extstats() now accumulates into 
pending entries with
pgstat_prep_pending_entry() for both PGSTAT_KIND_VACUUM_RELATION and 
PGSTAT_KIND_VACUUM_DB,
which pgstat_report_stat() then flushes through the registered 
pgstat_vacuum_relation_flush_cb() /
pgstat_vacuum_db_flush_cb().

(relation pending in patch 0001; database pending in patch 0003)
> 5) Commit message of 0001 uses stale names
> rev_all_visible_pages/rev_all_frozen_pages counters for
> visible_page_marks_cleared` / `frozen_page_marks_cleared.

Fixed.

A few other changes in this version, beyond the five points above:

  * I split the patch set so that each commit introduces one category of
    statistics.
    In particular, recently_dead_tuples now lives in the same commit as
    missed_dead_tuples (patch 0003), where
    it logically belongs - both are dead-tuple counters derived the same
    way, so keeping them together makes
    the series easier to review.
  * I also changed the order in which the counters are added across the
    commits. I'm still working out the right way
    to verify the buffer statistics, timings and he frozen /
    visibility-map counters.

Thanks again for the review.

-- 
-----------
Best regards,
Alena Rybakina,
Yandex Cloud

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add relallfrozen to pg_class

  2. Move wal_buffers_full from PgStat_PendingWalStats to WalUsage