public inbox for [email protected]
help / color / mirror / Atom feedFrom: Sami Imseih <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Daniil Davydov <[email protected]>
Subject: Re: test_autovacuum/001_parallel_autovacuum is broken
Date: Thu, 9 Apr 2026 15:14:15 -0500
Message-ID: <CAA5RZ0uOSnAKX4xF0PBSZqYCxUMTnheAKkcsiQDNYTZg20ognQ@mail.gmail.com> (raw)
In-Reply-To: <CAD21AoBte7CSVRpnOvXibYPsqOE9aswsp7jrQW+8fhwxRZFWpg@mail.gmail.com>
References: <CAA5RZ0s+kZZRMSF4HW7tZ9W2jS1o4B+Fg8dr5a-T6mANX+mdQA@mail.gmail.com>
<[email protected]>
<CAD21AoCj=OSxoh3RBw0VWJ7Y7c9EipRhpatcZaHuXKGLWyKgdQ@mail.gmail.com>
<CAA5RZ0uqvjxv5HPzz36UEq-LUf8F+kae61k0imaGw0C0XGa7nw@mail.gmail.com>
<CAD21AoBte7CSVRpnOvXibYPsqOE9aswsp7jrQW+8fhwxRZFWpg@mail.gmail.com>
> Optionally, we can change the comment above to something like this:
> # Wait until the parallel autovacuum on the table completes and reports the
> # number of launched workers, which must correspond to the value specified in
> # the reloption.
Made the comment less verbose but in the same spirit as the above.
> > > One way to fix the test is to replace log_contains() with
> > > wait_for_log(). We can also remove wait_for_autovacuum_complete()
> > > logic altogether.
> >
> > +1. I was going to reply with exactly this. Attached is the fix.
>
> Thank you for the patch! I agree with the overall idea. Since we
> enable autovacuum log only the test_autovac table, just checking
> autovacuum log works as expected.
>
> I think we can simplify the test further by removing the logic around
> the av_count variable.
removed av_count and pg_stat_user_tables query, but hardened the
regexp a bit to ensure that the parallel logging is for the test_autovac
table. It gives the same assurance as counting pg_stat_user_tables
and will be better if we add another parallel test table in the future.
--
Sami
Attachments:
[application/octet-stream] v2-0001-Fix-unstable-log_contains-in-parallel-autovacuum-.patch (3.2K, 2-v2-0001-Fix-unstable-log_contains-in-parallel-autovacuum-.patch)
download | inline diff:
From dfd011b45a8880104372d7f24f4e85a27252ed17 Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Thu, 9 Apr 2026 20:06:00 +0000
Subject: [PATCH v2 1/1] Fix unstable log_contains in parallel autovacuum tests
Replace log_contains with wait_for_log in the parallel autovacuum
test to ensure we wait for the log containing parallel vacuum info.
Remove wait_for_autovacuum_complete as the log wait is sufficient
to determine vacuum completion. The query to pg_stat_user_tables
for vacuum count is also no longer needed since we now ensure the
parallel logging is associated with the test table.
---
.../t/001_parallel_autovacuum.pl | 33 ++++---------------
1 file changed, 6 insertions(+), 27 deletions(-)
diff --git a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl
index fc4dd22f5eb..206c95f9157 100644
--- a/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl
+++ b/src/test/modules/test_autovacuum/t/001_parallel_autovacuum.pl
@@ -15,8 +15,7 @@ if ($ENV{enable_injection_points} ne 'yes')
}
# Before each test we should disable autovacuum for 'test_autovac' table and
-# generate some dead tuples in it. Returns the current autovacuum_count of
-# the table test_autovac.
+# generate some dead tuples in it.
sub prepare_for_next_test
{
my ($node, $test_number) = @_;
@@ -26,24 +25,6 @@ sub prepare_for_next_test
ALTER TABLE test_autovac SET (autovacuum_enabled = false);
UPDATE test_autovac SET col_1 = $test_number;
});
-
- my $count = $node->safe_psql(
- 'postgres', qq{
- SELECT autovacuum_count FROM pg_stat_user_tables WHERE relname = 'test_autovac'
- });
-
- return $count;
-}
-
-# Wait for the table to be vacuumed by an autovacuum worker.
-sub wait_for_autovacuum_complete
-{
- my ($node, $old_count) = @_;
-
- $node->poll_query_until(
- 'postgres', qq{
- SELECT autovacuum_count > $old_count FROM pg_stat_user_tables WHERE relname = 'test_autovac'
- });
}
my $node = PostgreSQL::Test::Cluster->new('main');
@@ -119,7 +100,7 @@ $node->safe_psql(
# Our table has enough indexes and appropriate reloptions, so autovacuum must
# be able to process it in parallel mode. Just check if it can do it.
-my $av_count = prepare_for_next_test($node, 1);
+prepare_for_next_test($node, 1);
my $log_offset = -s $node->logfile;
$node->safe_psql(
@@ -127,18 +108,16 @@ $node->safe_psql(
ALTER TABLE test_autovac SET (autovacuum_enabled = true);
});
-# Wait until the parallel autovacuum on table is completed. At the same time,
-# we check that the required number of parallel workers has been started.
-wait_for_autovacuum_complete($node, $av_count);
-ok( $node->log_contains(
- qr/parallel workers: index vacuum: 2 planned, 2 launched in total/,
+# Wait for parallel autovacuum to complete; check worker count matches reloptions.
+ok( $node->wait_for_log(
+ qr/automatic vacuum of table ".*?\..*?\.test_autovac":.*?parallel workers: index vacuum: 2 planned, 2 launched in total/s,
$log_offset));
# Test 2:
# Check whether parallel autovacuum leader can propagate cost-based parameters
# to the parallel workers.
-$av_count = prepare_for_next_test($node, 2);
+prepare_for_next_test($node, 2);
$log_offset = -s $node->logfile;
$node->safe_psql(
--
2.50.1
view thread (20+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: test_autovacuum/001_parallel_autovacuum is broken
In-Reply-To: <CAA5RZ0uOSnAKX4xF0PBSZqYCxUMTnheAKkcsiQDNYTZg20ognQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox