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 |
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 |