Re: Vacuum statistics

Hi! I updated patch.

On 02.12.2024 23:12, Alena Rybakina wrote:
> 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.

I propose to solve this with a guc (track_vacuum_statistics) that will 
allow us to disable statistics collection or enable it back when needed. 
This statistics is useful if you collect it over a certain period of 
time and watch the dynamics of the change, so I think a hook will not 
hurt here.

I also added new statistics vm_new_visible_frozen_pages due to the 
dc6acfd commit and renamed some of my statistics from frozen_pages to 
vm_new_frozen_pages and all-visible_pages to vm_new_visible_pages. I 
also added statistics missed_tupes, missed_pages. Both are necessary to 
take into account how many tuples were not cleared by vacuum due to 
failure to acquire a cleanup lock on a heap page. The second statistics 
missed_pages will allow you to track whether this is one particular page 
or not.

This information can make it clear that perhaps the data is broken 
somewhere or there is an error in the operation of the database, for 
example.

>> * 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.
Fixed
>> * Commit message for 0001 looks nice, but commit messages of 0002,
>> 0003, and 0004 look messy.  Could you please, rearrange them.
Fixed
>> * 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?
Fixed
>> * 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.
Fixed
>> * 0004 should be pure documentation patch, but it seems containing
>> changes to isolation tests.  Please, move them into a more appropriate
>> place.
>>
Fixed


Thanks for your review, it was very helpful)

I also noticed that my stats for indexes were not being collected while 
parallel vacuum was running. I fixed it by adding some extra code that 
basically captured changes to the parallel_vacuum_process_all_indexes 
function. I used a script like this to check if everything was correct.

pgbench -d postgres -i -s 10


my/inst/bin/pg_basebackup -D ~/backup


#psql

--check parallel vacuum statistics

create index accounts_idx1 on pgbench_accounts(bid);
create index accounts_idx2 on pgbench_accounts(aid, bid);

delete from pgbench_accounts where aid >5;
set max_parallel_maintenance_workers = 8;
VACUUM (PARALLEL 3) pgbench_accounts;


create index accounts_idx1 on pgbench_accounts(bid);
create index accounts_idx2 on pgbench_accounts(aid, bid);

delete from pgbench_accounts where aid >5;
set max_parallel_maintenance_workers = 8;
VACUUM (PARALLEL 3) pgbench_accounts;


pg_ctl -D ../postgres_data11 -l logfile stop

rm -rf ../postgres_data/*

cp -r ~/backup/* ~/postgres_data/

pg_ctl -D ../postgres_data11 -l logfile start


--check vacuum statistics processed by postmaster only

create index accounts_idx1 on pgbench_accounts(bid);
create index accounts_idx2 on pgbench_accounts(aid, bid);

delete from pgbench_accounts where aid >5;
set max_parallel_maintenance_workers = 8;
VACUUM (PARALLEL 0) pgbench_accounts;


To view statistics:

select vt.relname, total_blks_read AS total_blks_read,
   total_blks_hit AS total_blks_hit,
   total_blks_dirtied AS total_blks_dirtied,
   total_blks_written AS total_blks_written,

   rel_blks_read AS rel_blks_read,
   rel_blks_hit AS rel_blks_hit,

   pages_deleted AS pages_deleted,
   tuples_deleted AS tuples_deleted,

   wal_records AS wal_records,
   wal_fpi AS wal_fpi,
   wal_bytes AS wal_bytes,

   blk_read_time AS blk_read_time,
   blk_write_time AS blk_write_time,

   delay_time AS delay_time,
   total_time AS total_time
   FROM pg_stat_get_vacuum_indexes vt, pg_class c
WHERE (vt.relname='accounts_idx1' or vt.relname='accounts_idx2' or 
vt.relname = 'pgbench_accounts_pkey') AND vt.relid = c.oid;


select stats.relname,stats.total_blks_read AS total_blks_read,
   stats.total_blks_hit AS total_blks_hit,
   stats.total_blks_dirtied AS total_blks_dirtied,
   stats.total_blks_written AS total_blks_written,

   stats.rel_blks_read AS rel_blks_read,
   stats.rel_blks_hit AS rel_blks_hit,

   stats.pages_scanned AS pages_scanned,
   stats.pages_removed AS pages_removed,
   stats.pages_frozen AS pages_frozen,
   stats.pages_all_visible AS pages_all_visible,
   stats.tuples_deleted AS tuples_deleted,
   stats.tuples_frozen AS tuples_frozen,
   stats.dead_tuples AS dead_tuples,

   stats.index_vacuum_count AS index_vacuum_count,
   stats.wal_records AS wal_records,
   stats.wal_fpi AS wal_fpi,
   stats.wal_bytes AS wal_bytes,

   stats.blk_read_time AS blk_read_time,
   stats.blk_write_time AS blk_write_time,

   stats.delay_time AS delay_time,
   stats.total_time AS total_time from pg_stat_vacuum_tables stats, 
pg_stat_all_tables  WHERE stats.relname = 'pgbench_accounts' and 
stats.relid = pg_stat_all_tables.relid;


output_single_19I got the following results and stored them in 
output_single_19 and output_parallel_19 files.
I noticed that rel_blks_read and rel_blks_hit are too small compared to 
the vacuum statistics when the vacuum is not parallel. I suspect that 
this problem is related to the fact that the relationship statistics 
have not reached that time. You can see that they are calculated in my 
patch like this:

report->blks_fetched =
     rel->pgstat_info->counts.blocks_fetched - counters->blocks_fetched;
   report->blks_hit =
     rel->pgstat_info->counts.blocks_hit - counters->blocks_hit;


The second thing that bothered me was that some table stats differ in 
the fields total_blks_read, rel_blks_read, pages_removed. If with the 
buffer this could be related to the fact that in a single run we rely on 
the stats of the global buffer and shaft statistics and this could 
explain why there are more of them, then with pages_removed I have no 
explanation yet as to what could have happened. I am still studying this.

When you have time, take a look at the patches, I will be glad to 
receive any feedback.

-- 
Regards,
Alena Rybakina
Postgres Professional