| From: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
|---|---|
| To: | Alena Rybakina <lena(dot)ribackina(at)yandex(dot)ru> |
| Cc: | Andrei Lepikhov <lepihov(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Jim Nasby <jnasby(at)upgrade(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Andrei Zubkov <zubkov(at)moonset(dot)ru>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, Sami Imseih <samimseih(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com> |
| Subject: | Re: Vacuum statistics |
| Date: | 2026-03-15 17:18:28 |
| Message-ID: | 10BE6E39-94D2-4909-ACB8-24E1FA6580EE@yandex-team.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 13 Mar 2026, at 18:04, Alena Rybakina <lena(dot)ribackina(at)yandex(dot)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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mahendra Singh Thalor | 2026-03-15 17:29:53 | pg_restore: TAP test case typo(wrong word) for an error hint in 001_basic.pl |
| Previous Message | Mahendra Singh Thalor | 2026-03-15 17:02:11 | Re: pg_restore --format= option(without any value) should report an error as pg_dump is reporting an error |