Re: Fix parallel vacuum buffer usage reporting

On 22.04.2024 11:07, Anthonin Bonnefoy wrote:
> On Sat, Apr 20, 2024 at 2:00 PM Alena Rybakina 
> <lena.ribackina@yandex.ru> wrote:
>
>     Hi, thank you for your work with this subject.
>
>     While I was reviewing your code, I noticed that your patch
>     conflicts with another patch [0] that been committed.
>
>     [0]
>     https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com
>
>
> I've rebased the patch and also split the changes:
> 1: Use pgBufferUsage in Vacuum and Analyze block reporting
> 2: Stop tracking buffer usage in the now unused Vacuum{Hit,Miss,Dirty} 
> variables
> 3: Remove declarations of Vacuum{Hit,Miss,Dirty}

Hi! Thank you for your work, and I have reviewed your corrections.

I tested the main postgres branch with and without your fix using a 
script that was written by me. It consists of five scenarios and I made 
a comparison in the logs between the original version of the master 
branch and the master branch with your patch:

1. I added 1 million data to the table and deleted 10% of them. I ran 
vacuum verbose and didn't see any differences.
buffer usage: 12585 hits, 0 misses, 4 dirtied
2. I opened another connection with a repeatable read transaction 
through the dblink extension and launched a query updating the records 
in the table under test. Later, I ran vacuum verbose again and also 
didn't see any differences.
buffer usage: 19424 hits, 0 misses, 6 dirtied
3. I commited transaction and ran vacuum verbose again. Everything is 
fine in the logs.
buffer usage: 22960 hits, 0 misses, 11456 dirtied
4.  I deleted all the data from the table and later started vacuum 
verbose again. The number of pages in the buffer matched with your patch 
too.
5.I inserted 1 million data into the table and updated it, and I found 
the difference between the original master version and the version with 
your patch:
with your patch: buffer usage: 32315 hits, 606 misses, 1547 dirtied
original version: buffer usage: 32348 hits, 573 misses, 1456 dirtied
I suppose, something wasn't calculated.

The same script was run, but using vacuum verbose analyze, and I saw the 
difference again in the fifth step:
with your patch: buffer usage: 32312 hits, 607 misses, 1566 dirtied
master: buffer usage: 32346 hits, 573 misses, 1360 dirtied

I have attached a test file (vacuum_check_logs.sql) and four log files: 
two with the vacuum verbose command (vacuum_log and 
vacuum_log_with_patch files) and two others with the vacuum verbose 
analyze command (vacuum_analyze_test_with_patch and vacuum_analyze_test 
files). Both test approaches show logs with and without your changes.
>> 1: Use pgBufferUsage in Vacuum and Analyze block reporting
> I think that if the anayze command doesn't have the same issue, we
> don't need to change it. Making the vacuum and the analyze consistent
> is a good point but I'd like to avoid doing unnecessary changes in
> back branches. I think the patch set would contain:
>
> (a) make lazy vacuum use BufferUsage instead of
> VacuumPage{Hit,Miss,Dirty}. (backpatched down to pg13).
> (b) make analyze use BufferUsage and remove VacuumPage{Hit,Miss,Dirty}
> variables for consistency and simplicity (only for HEAD, if we agree).
>
> BTW I realized that VACUUM VERBOSE running on a temp table always
> shows the number of dirtied buffers being 0, which seems to be a bug.
> The patch (a) will resolve it as well.
>
I agree with that. I think we can leave these changes to the analysis 
command for master, but I also doubt the need to backport his changes to 
back versions.

-- 
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company