Re: Using read_stream in index vacuum

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
Cc: Kirill Reshke <reshkekirill(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, Junwang Zhao <zhjwpku(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: Using read_stream in index vacuum
Date: 2025-03-21 00:54:37
Message-ID: CAAKRu_YeMT67LfyN7siwiACP9Q7YnCY9zdzxbv1Rhkeg94mYSQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 19, 2025 at 5:26 AM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
> So, yes, your change to the test seems correct to me. We can do the test with just one injection point.

Attached 0001 is what I plan to commit first thing tomorrow morning. I
moved the vacuum_delay_point() so that we would call
pgstat_progress_update_param() as soon as we were done processing the
block (instead of doing vacuum_delay_point() first). This also makes
it consistent with the other vacuum streaming read users. What do we
even use the progress update for here, though? For heap vacuuming it
makes sense because we report heap blocks vacuumed in
pg_stat_progress_vacuum, but we don't do that for index blocks
vacuumed...

I plan to commit 0001 (the read stream user) without 0002 and then
solicit more feedback on 0002.

0002 is a draft of the test. I want us to actually test the
backtracking behavior. To do this, I think we need two injection
points with the current injection point infrastructure. Which I don't
love. I think if we could pass variables into INJECTION_POINT(), we
could do the test with one injection point.

I also don't love how "failing" for this test is just it hanging
because the nbtree-vacuum-page-backtrack wait event never happens
(there is no specific assertion or anything in the test).

I do like that I moved the nbtree-vacuum-page injection point to the
top of nbtreevacuumpage because I think it can be used for other tests
in the future.

I also think it is worth making a btree directory in src/test instead
of adding this to src/test/modules/test_misc. In fact it might be
worth moving the gin and brin tests out of src/test/modules and making
a new "indexes" directory in src/test with gin, brin, and btree (also
some spgist tests are in there somewhere) subdirectories.

- Melanie

Attachment Content-Type Size
v10-0002-test-backtrack.patch text/x-patch 5.7 KB
v10-0001-Use-streaming-read-I-O-in-btree-vacuuming.patch text/x-patch 7.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-03-21 00:54:55 Re: query_id: jumble names of temp tables for better pg_stat_statement UX
Previous Message David Rowley 2025-03-21 00:41:09 Re: Remove redundant if-else in EXPLAIN by using ExplainPropertyText