Content-Type: multipart/alternative;
 boundary="------------Gcn1y04SICcD0p2ihyQCLWRP"
Message-ID: <30d54302-9e9c-4e04-819e-a13b679cdcc8@postgrespro.ru>
Date: Mon, 2 Dec 2024 23:12:05 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Vacuum statistics
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Jim Nasby <jnasby@upgrade.com>, Andrei Zubkov <zubkov@moonset.ru>,
 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
References: <bee34abe-fe87-49b3-8523-8ae19e641ccf@yandex.ru>
 <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>
 <CAPpHfds=woPcB9nPtMmu=g=U9q6-FHFh7fF_x=uhU3k2Oi03sA@mail.gmail.com>
Content-Language: en-US
From: Alena Rybakina <a.rybakina@postgrespro.ru>
In-Reply-To: <CAPpHfds=woPcB9nPtMmu=g=U9q6-FHFh7fF_x=uhU3k2Oi03sA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/30d54302-9e9c-4e04-819e-a13b679cdcc8%40postgrespro.ru>
Precedence: bulk

This is a multi-part message in MIME format.
--------------Gcn1y04SICcD0p2ihyQCLWRP
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 02.12.2024 11:27, Alexander Korotkov wrote:
> Hi, Alena!
>
> On Wed, Nov 13, 2024 at 6:21 PM Alena Rybakina
> <a.rybakina@postgrespro.ru>  wrote:
>> 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.
> Thank you for the updated patchset.  Some points from me.
>
> * I've read the previous discussion on how important to keep all these
> fields regarding vacuum statistics including points by Andrei and Jim.
> It still worrying me that statistics volume is going to burst in about
> 3 times, but I don't have a particular proposal on how to make more
> granular approach.  I wonder if you could propose something.
> * Previously PGSTAT_FILE_FORMAT_ID got increased by 1. Your 0001 patch
> increases it by 2.  It's minor note, but I'd like to keep the
> tradition.
> * Commit message for 0001 looks nice, but commit messages of 0002,
> 0003, and 0004 look messy.  Could you please, rearrange them.
> * The distinction between 0001 and 0002 is not clear. The first line
> of 0001 is "Machinery for grabbing an extended vacuum statistics on
> heap relations", the first line of 0002 is "Machinery for grabbing an
> extended vacuum statistics on heap and index relations."  I guess 0001
> should be about heap relations while 0002 should be about just index
> relations.  Is this correct?
> * I guess this statistics should work for any table AM, based on what
> has been done in relation_vacuum() interface method.  If that's
> correct, we need to get rid of "heap" terminology and use "table"
> instead.
> * 0004 should be pure documentation patch, but it seems containing
> changes to isolation tests.  Please, move them into a more appropriate
> place.
>
Thank you for your valuable feedback, I am already carefully processing 
your comments and will update the patches soon.

I will think about what can be done to address the problem of increasing 
the volume of statistics; perhaps it will be possible to implement a guc 
that, when enabled, will accumulate additional information on vacuum 
statistics. For example, this way you can group statistics by buffers 
and vacuum statistics.

-- 
Regards,
Alena Rybakina
Postgres Professional

--------------Gcn1y04SICcD0p2ihyQCLWRP
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    On 02.12.2024 11:27, Alexander Korotkov wrote:<br>
    <blockquote type="cite"
cite="mid:CAPpHfds=woPcB9nPtMmu=g=U9q6-FHFh7fF_x=uhU3k2Oi03sA@mail.gmail.com">
      <pre class="moz-quote-pre" wrap="">Hi, Alena!

On Wed, Nov 13, 2024 at 6:21 PM Alena Rybakina
<a class="moz-txt-link-rfc2396E" href="mailto:a.rybakina@postgrespro.ru">&lt;a.rybakina@postgrespro.ru&gt;</a> wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">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.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Thank you for the updated patchset.  Some points from me.

* I've read the previous discussion on how important to keep all these
fields regarding vacuum statistics including points by Andrei and Jim.
It still worrying me that statistics volume is going to burst in about
3 times, but I don't have a particular proposal on how to make more
granular approach.  I wonder if you could propose something.
* Previously PGSTAT_FILE_FORMAT_ID got increased by 1. Your 0001 patch
increases it by 2.  It's minor note, but I'd like to keep the
tradition.
* Commit message for 0001 looks nice, but commit messages of 0002,
0003, and 0004 look messy.  Could you please, rearrange them.
* The distinction between 0001 and 0002 is not clear. The first line
of 0001 is "Machinery for grabbing an extended vacuum statistics on
heap relations", the first line of 0002 is "Machinery for grabbing an
extended vacuum statistics on heap and index relations."  I guess 0001
should be about heap relations while 0002 should be about just index
relations.  Is this correct?
* I guess this statistics should work for any table AM, based on what
has been done in relation_vacuum() interface method.  If that's
correct, we need to get rid of "heap" terminology and use "table"
instead.
* 0004 should be pure documentation patch, but it seems containing
changes to isolation tests.  Please, move them into a more appropriate
place.

<span style="white-space: pre-wrap">
</span></pre>
    </blockquote>
    Thank you for your valuable feedback, I am already carefully
    processing your comments and will update the patches soon.<br>
    <br>
    I will think about what can be done to address the problem of
    increasing the volume of statistics; perhaps it will be possible to
    implement a guc that, when enabled, will accumulate additional
    information on vacuum statistics. For example, this way you can
    group statistics by buffers and vacuum statistics.
    <pre class="moz-signature" cols="72">-- 
Regards,
Alena Rybakina
Postgres Professional</pre>
  </body>
</html>

--------------Gcn1y04SICcD0p2ihyQCLWRP--