MIME-Version: 1.0
References: <bee34abe-fe87-49b3-8523-8ae19e641ccf@yandex.ru>
 <c4e4e305-7119-4183-b49a-d7092f4efba3@postgrespro.ru> <f05a9031-f923-41b1-9bf9-81204cb56561@postgrespro.ru>
 <CACJufxFU4ej3iGtGg3GDqmGzRmTVq3d9RGq+ibLfQoS8E3hJEQ@mail.gmail.com>
 <9b10c6d3-52c4-4eef-b67c-c33442667729@postgrespro.ru> <CAD21AoAVK7DwTZLfhwuRhTGgR=_ASu5YshEg_Cmpojk5ZdZ3tA@mail.gmail.com>
 <CAAKRu_auu=xt4w3Mm_jW-voJunZgno6XDDohH6hhRc4Z9dGdYQ@mail.gmail.com>
 <CAD21AoD66b3u28n=73kudgMp5wiGiyYUN9LuC9z2ka6YTru5Gw@mail.gmail.com>
 <9485d892-fd04-4e3a-ac24-7dd767cb7333@postgrespro.ru> <CAPpHfduwY8-fp34CuO9O57ouCs1K=Gn1rTnuG4AaWYhEo6nXyw@mail.gmail.com>
 <0B6CBF4C-CC2A-4200-9126-CE3A390D938B@upgrade.com> <6732acf8ce0f31025b535ae1a64568750924a887.camel@moonset.ru>
 <5AA8FFD5-6DE2-4A31-8E00-AE98F738F5D1@upgrade.com> <85b963fe-5977-43aa-9241-75b862abcc69@postgrespro.ru>
 <E0B6D242-C16D-4848-930E-B693D4E5C678@upgrade.com> <e2003f94-2075-4183-8298-9d25d8431fb1@postgrespro.ru>
 <9C7A167C-DCDE-4A17-9ABE-6276723FEC50@upgrade.com> <2d493cf9-9ba7-4cc1-a3f2-67afd7c163ee@postgrespro.ru>
In-Reply-To: <2d493cf9-9ba7-4cc1-a3f2-67afd7c163ee@postgrespro.ru>
From: Kirill Reshke <reshkekirill@gmail.com>
Date: Sat, 30 Nov 2024 09:48:22 +0500
Message-ID: <CALdSSPhv1MAd9GPyWcnMxxP2ZgXFpCksqe7onbkVzQwLaySqgw@mail.gmail.com>
Subject: Re: Vacuum statistics
To: Alena Rybakina <a.rybakina@postgrespro.ru>
Cc: Jim Nasby <jnasby@upgrade.com>, Andrei Zubkov <zubkov@moonset.ru>, 
	Alexander Korotkov <aekorotkov@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, 
	Melanie Plageman <melanieplageman@gmail.com>, jian he <jian.universality@gmail.com>, 
	pgsql-hackers <pgsql-hackers@postgresql.org>, a.lepikhov@postgrespro.ru
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CALdSSPhv1MAd9GPyWcnMxxP2ZgXFpCksqe7onbkVzQwLaySqgw%40mail.gmail.com>
Precedence: bulk

On Wed, 13 Nov 2024 at 21:21, Alena Rybakina <a.rybakina@postgrespro.ru> wr=
ote:
>
> Hi! Thank you for your contribution to this thread!
>
> On 13.11.2024 03:24, Jim Nasby wrote:
>
> On Nov 10, 2024, at 2:09=E2=80=AFPM, Alena Rybakina <a.rybakina@postgresp=
ro.ru> wrote:
>
>
> On 08.11.2024 22:34, Jim Nasby wrote:
>
>
> On Nov 2, 2024, at 7:22=E2=80=AFAM, Alena Rybakina <a.rybakina@postgrespr=
o.ru> wrote:
>
> The second is the interrupts field. It is needed for monitoring to know
> do we have them or not, so tracking them on the database level will do
> the trick. Interrupt is quite rare event, so once the monitoring system
> will catch one the DBA can go to the server log for the details.
>
> Just to confirm=E2=80=A6 by =E2=80=9Cinterrupt=E2=80=9D you mean vacuum e=
ncountered an error?
>
> Yes it is.
>
> In that case I feel rather strongly that we should label that as =E2=80=
=9Cerrors=E2=80=9D. =E2=80=9CInterrupt=E2=80=9D could mean a few different =
things, but =E2=80=9Cerror=E2=80=9D is very clear.
>
> I updated patches. I excluded system and user time statistics and save nu=
mber of interrupts only for database. I removed the ability to get statisti=
cs for all tables, now they can only be obtained for an oid table [0], as s=
uggested here. I also renamed the statistics from pg_stat_vacuum_tables to =
pg_stat_get_vacuum_tables and similarly for indexes and databases. I notice=
d that that=E2=80=99s what they=E2=80=99re mostly called. Ready for discuss=
ion.
>
> I think it=E2=80=99s better that the views follow the existing naming con=
ventions (which don=E2=80=99t include =E2=80=9C_get_=E2=80=9D; only the fun=
ctions have that in their names). Assuming that, the only question becomes =
pg_stat_vacuum_* vs pg_stat_*_vacuum. Given the existing precedent of pg_st=
atio_*, I=E2=80=99m inclined to go with pg_stat_vacuum_*.
>
> I have fixed it.
>
>
> I=E2=80=99ve reviewed and made some cosmetic changes to patch 1, though o=
f note it looks like an effort has been made to keep stat_reset_timestamp a=
t the end of PgStat_StatDBEntry, so I re-arranged that. I also removed some=
 obviously dead code. It appears that pgstat_update_snapshot(), InitSnapsho=
tIterator() and ScanStatSnapshot() are also dead, but I=E2=80=99ve left it =
in incase I=E2=80=99m missing something. The tests are also failing for me =
because a number of psql variables aren=E2=80=99t set.
>
> Thank you! Yes, I have deleted them.
>
>
> I do think we should separate out the counts for deleted but still visibl=
e tuples vs tuples where we couldn=E2=80=99t get a cleanup lock (in other w=
ords, recently_dead_tuples and missed_dead_tuples from LVRelState). I reali=
ze that=E2=80=99s a departure from how some of the existing reporting works=
, but IMO combining them together isn=E2=80=99t a pattern we should be repe=
ating since they mean completely different things. Towards that end I did r=
emove missed_dead_tuples from the reporting, and renamed ExtVacReport.dead_=
tuples to recently_dead_tuples, but I stopped short of creating a separate =
entry for missed_dead_tuples. Note that while recently_dead_tuples is reall=
y a global thing (so only needs to be reported at a global (or at most per-=
database) level, but missed_dead_tuples should really be at a per-table lev=
el.
>
> I am willing to agree with your idea. But we need to think about how clea=
rly describe them in the documentation.
>
>
> Updated 0001-v13 attached, as well as the diff between v12 and v13.
>
> Thank you)
>
> And I agree with your changes. And included them in patches.
>
> ---
> Regards,
> Alena Rybakina
> Postgres Professional

Hello!
After a brief glance, I think this patch set is good.
But there isn't any more time in the current CF to commit this :(.
So I moved to the next CF.

I also like the 0001 commit message. This commit message is quite
large and easy to understand. Actually, it might be too big. Perhaps
rather of being a commit message, the final paragraph (pages_frozen -
number of pages that..) need to be a part of the document. Perhaps
delete the explanation on pages_frozen that we have in 0004?


--=20
Best regards,
Kirill Reshke