Re: Using read_stream in index vacuum

On Thu, Nov 28, 2024 at 10:29 AM Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>
> 0001 Looks mature. Some comments:
> 1)
> >+# This ensures autovacuum do not run
> >+$node->append_conf('postgresql.conf', 'autovacuum = off');
> The other option is to set `autovacuum = off `in relation DDL. I'm not
> sure which option is better.

Either is fine. Though perhaps it is better to turn it off globally in
this case since we might as well avoid wasting any resources.

> 2) Are these used?
> my $psql_err = '';
> my $psql_out = '';

They don't seem to be.

I'm looking at 0001 with the intent of committing it soon. Today I've
just been studying the test with the injection points.

My main thought is that you should rename the injection points to
something more descriptive. We want to make it clear why there are
two. Also nbtree-vacuum-2 is actually where we wait the first
iteration of the loop, so that is confusing.

Perhaps we also ought to pass parallel false in the vacuum command.

I spent time thinking about if there was a way to exercise the actual
backtracking code and test that index entries that should have been
cleaned up were left behind because they were moved from a page we
didn't process before the split started to one we did. I thought maybe
we could do something with pageinspect. But getting the timing right
seems too hard.

Anyway, it seems worth it to have a bit of coverage for needing
multiple passes in btvacuumscan(). And given the injection points
infrastructure, perhaps other btree vacuum tests could be added to
this file in the future.

- Melanie