On Thu, Nov 28, 2024 at 10:29 AM Kirill Reshke <reshkekir...@gmail.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
