Re: Vacuum statistics

> On 13 Mar 2026, at 18:04, Alena Rybakina <lena.ribackina@yandex.ru> wrote:

I've decided to take a look into v31.

Overall idea of tracking VM dynamics seems good to me.

But the column naming for rev_all_visible_pages and rev_all_frozen_pages
seems strange to me. I've skimmed the thread but could not figure out what
"rev_" stands for. Revisions? Revolutions? Reviews?

Is there a reason why you break "SELECT * FROM pg_stat_all_tables" for
an existing software? IMO even if we want these columns in this exact view
- they ought to be appended to the end of the column list.

Some nits about the code.

my $interval   = 0.015;
sleep($interval); <--- sleep takes integer AFAIK?

Maybe just use poll_query_until()?

$start_time seems unused.

I don't think src/test/recovery/t/ is good for the test. It has nothing to
do with recovery. Maybe somewhere in src/test/modules?

This change is not needed at all:
- proargtypes => 'oid8 int8', prosrc => 'hashoid8extended' },
+ proargtypes => 'oid8 int8', prosrc => 'hashoid8extended' },

s/'statistics: number of times the all-visible pages in the visibility map
was removed for pages of table'/'statistics: number of times the all-visible
pages in the visibility map were cleared for pages of this table'/g

I would appreciate some braces in
if (map[mapByte] >> mapOffset & flags & VISIBILITYMAP_ALL_VISIBLE)
Probably the code is correct, but I write in languages with different parsers
and do not trust in grammar priorities. Is it something like following?
if (map[mapByte] & ((VISIBILITYMAP_ALL_VISIBLE & flags) << mapOffset))
We check (map[mapByte] & mask) in this if statement which is flags << mapOffset btw...


That's all what catches my eye this time. Thank you!


Best regards, Andrey Borodin.