Re: Fix parallel vacuum buffer usage reporting

Hi,

Thank you for the report.

On Fri, Feb 9, 2024 at 6:10 PM Anthonin Bonnefoy
<anthonin(dot)bonnefoy(at)datadoghq(dot)com> wrote:
>
> Hi,
>
> With a db setup with pgbench, we add an additional index:
> CREATE INDEX ON pgbench_accounts(abalance)
>
> And trigger several updates and vacuum to reach a stable amount of
> dirtied pages:
> UPDATE pgbench_accounts set abalance = abalance + 1 WHERE aid=1; CHECKPOINT;
> VACUUM (VERBOSE, INDEX_CLEANUP ON) pgbench_accounts
>
> The vacuum will report the following:
> INFO: vacuuming "postgres.public.pgbench_accounts"
> INFO: launched 1 parallel vacuum worker for index vacuuming (planned: 1)
> INFO: finished vacuuming "postgres.public.pgbench_accounts": index scans: 1
> ...
> buffer usage: 122 hits, 165 misses, 4 dirtied
>
> 4 pages were reported dirtied. However, we have 5 dirtied blocks at
> the end of the vacuum when looking at pg_buffercache:
>
> SELECT c.relname, b.relfilenode
> FROM
> pg_buffercache b LEFT JOIN pg_class c
> ON b.relfilenode =
> pg_relation_filenode(c.oid)
> WHERE isdirty=true;
> relname | relfilenode
> -------------------------------+-------------
> pg_class | 1259
> pgbench_accounts | 16400
> pgbench_accounts | 16400
> pgbench_accounts_pkey | 16408
> pgbench_accounts_abalance_idx | 16480
>
> The missing dirty block comes from the parallel worker vacuuming the
> abalance index. Running vacuum with parallel disabled will give the
> correct result.
>
> Vacuum uses dedicated VacuumPage{Hit,Miss,Dirty} globals to track
> buffer usage. However, those values are not collected at the end of
> parallel vacuum workers, leading to incorrect buffer count.

True. I think we should fix it also on backbranches.

>
> Those vacuum specific globals are redundant with the existing
> pgBufferUsage and only used in the verbose output. This patch removes
> them and replaces them by pgBufferUsage which is already correctly
> collected at the end of parallel workers, fixing the buffer count.

It seems to make sense to remove VacuumPageHit and friends, only on
the master branch, if we can use BufferUsage instead.

As for the proposed patch, the following part should handle the temp tables too:

appendStringInfo(&buf, _("avg read rate: %.3f
MB/s, avg write rate: %.3f MB/s\n"),
read_rate, write_rate);
appendStringInfo(&buf, _("buffer usage: %lld
hits, %lld misses, %lld dirtied\n"),
- (long long)
AnalyzePageHit,
- (long long)
AnalyzePageMiss,
- (long long)
AnalyzePageDirty);
+ (long long)
bufferusage.shared_blks_hit,
+ (long long)
bufferusage.shared_blks_read,
+ (long long)
bufferusage.shared_blks_dirtied);
appendStringInfo(&buf, _("system usage: %s"),
pg_rusage_show(&ru0));

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com