From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Odd, intermittent failure in contrib/pageinspect
Date: Tue, 19 Jan 2021 17:03:49 -0500
Message-ID: <550868.1611093829@sss.pgh.pa.us> (raw)
In-Reply-To: <YAZrAq+w50tAkpTt@paquier.xyz>
References: <20210118224005.GA3146@alvherre.pgsql>
<416876.1611010060@sss.pgh.pa.us>
<YAZrAq+w50tAkpTt@paquier.xyz>
Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Jan 18, 2021 at 05:47:40PM -0500, Tom Lane wrote:
>> So, do we have any other tests that are invoking a manual vacuum
>> and assuming it won't skip any pages? By this theory, they'd
>> all be failures waiting to happen.
> check_heap.sql and heap_surgery.sql have one VACUUM FREEZE each and it
> seems to me that we had better be sure that no pages are skipped for
> their cases?
It looks to me like heap_surgery ought to be okay, because it's operating
on a temp table; if there are any page access conflicts on that, we've
got BIG trouble ;-)
Poking around, I found a few other places where it looked like a skipped
page could produce diffs in the expected output:
contrib/amcheck/t/001_verify_heapam.pl
contrib/pg_visibility/sql/pg_visibility.sql
There are lots of other vacuums of course, but they don't look like
a missed page would have any effect on the visible results, so I think
we should leave them alone.
In short I propose the attached patch, which also gets rid of
that duplicate query.
regards, tom lane
Attachments:prevent-vacuum-page-skips-where-important.patch (text/x-diff, 8.1K) downloaddiff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.outindex 882f853d56..1fb3823142 100644--- a/contrib/amcheck/expected/check_heap.out+++ b/contrib/amcheck/expected/check_heap.out@@ -109,7 +109,7 @@ ERROR: ending block number must be between 0 and 0
SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 10000, endblock := 11000);
ERROR: starting block number must be between 0 and 0
-- Vacuum freeze to change the xids encountered in subsequent tests
-VACUUM FREEZE heaptest;+VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) heaptest;
-- Check that valid options are not rejected nor corruption reported
-- for a non-empty frozen table
SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'none');
diff --git a/contrib/amcheck/sql/check_heap.sql b/contrib/amcheck/sql/check_heap.sqlindex c10a25f21c..298de6886a 100644--- a/contrib/amcheck/sql/check_heap.sql+++ b/contrib/amcheck/sql/check_heap.sql@@ -51,7 +51,7 @@ SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 0, endblock :=
SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 10000, endblock := 11000);
-- Vacuum freeze to change the xids encountered in subsequent tests
-VACUUM FREEZE heaptest;+VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) heaptest;
-- Check that valid options are not rejected nor corruption reported
-- for a non-empty frozen table
diff --git a/contrib/amcheck/t/001_verify_heapam.pl b/contrib/amcheck/t/001_verify_heapam.plindex 1581e51f3c..a2f65b826d 100644--- a/contrib/amcheck/t/001_verify_heapam.pl+++ b/contrib/amcheck/t/001_verify_heapam.pl@@ -46,7 +46,7 @@ detects_heap_corruption(
# Check a corrupt table with all-frozen data
#
fresh_test_table('test');
-$node->safe_psql('postgres', q(VACUUM FREEZE test));+$node->safe_psql('postgres', q(VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) test));
corrupt_first_page('test');
detects_heap_corruption("verify_heapam('test')",
"all-frozen corrupted table");
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.outindex 4cd0db8018..4da28f0a1d 100644--- a/contrib/pageinspect/expected/page.out+++ b/contrib/pageinspect/expected/page.out@@ -1,7 +1,7 @@
CREATE EXTENSION pageinspect;
CREATE TABLE test1 (a int, b int);
INSERT INTO test1 VALUES (16777217, 131584);
-VACUUM test1; -- set up FSM+VACUUM (DISABLE_PAGE_SKIPPING) test1; -- set up FSM
-- The page contents can vary, so just test that it can be read
-- successfully, but don't keep the output.
SELECT octet_length(get_raw_page('test1', 'main', 0)) AS main_0;
@@ -87,18 +87,8 @@ SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
(1 row)
-- If we freeze the only tuple on test1, the infomask should
--- always be the same in all test runs. we show raw flags by--- default: HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID.-VACUUM FREEZE test1;-SELECT t_infomask, t_infomask2, raw_flags, combined_flags-FROM heap_page_items(get_raw_page('test1', 0)),- LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2);- t_infomask | t_infomask2 | raw_flags | combined_flags -------------+-------------+-----------------------------------------------------------+--------------------- 2816 | 2 | {HEAP_XMIN_COMMITTED,HEAP_XMIN_INVALID,HEAP_XMAX_INVALID} | {HEAP_XMIN_FROZEN}-(1 row)---- output the decoded flag HEAP_XMIN_FROZEN instead+-- always be the same in all test runs.+VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) test1;
SELECT t_infomask, t_infomask2, raw_flags, combined_flags
FROM heap_page_items(get_raw_page('test1', 0)),
LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2);
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sqlindex 01844cb629..d333b763d7 100644--- a/contrib/pageinspect/sql/page.sql+++ b/contrib/pageinspect/sql/page.sql@@ -3,7 +3,7 @@ CREATE EXTENSION pageinspect;
CREATE TABLE test1 (a int, b int);
INSERT INTO test1 VALUES (16777217, 131584);
-VACUUM test1; -- set up FSM+VACUUM (DISABLE_PAGE_SKIPPING) test1; -- set up FSM
-- The page contents can vary, so just test that it can be read
-- successfully, but don't keep the output.
@@ -34,15 +34,9 @@ SELECT tuple_data_split('test1'::regclass, t_data, t_infomask, t_infomask2, t_bi
SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
-- If we freeze the only tuple on test1, the infomask should
--- always be the same in all test runs. we show raw flags by--- default: HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID.-VACUUM FREEZE test1;+-- always be the same in all test runs.+VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) test1;-SELECT t_infomask, t_infomask2, raw_flags, combined_flags-FROM heap_page_items(get_raw_page('test1', 0)),- LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2);---- output the decoded flag HEAP_XMIN_FROZEN instead
SELECT t_infomask, t_infomask2, raw_flags, combined_flags
FROM heap_page_items(get_raw_page('test1', 0)),
LATERAL heap_tuple_infomask_flags(t_infomask, t_infomask2);
diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.outindex 0017e3415c..315633bfea 100644--- a/contrib/pg_visibility/expected/pg_visibility.out+++ b/contrib/pg_visibility/expected/pg_visibility.out@@ -105,7 +105,7 @@ ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
create table regular_table (a int, b text);
alter table regular_table alter column b set storage external;
insert into regular_table values (1, repeat('one', 1000)), (2, repeat('two', 1000));
-vacuum regular_table;+vacuum (disable_page_skipping) regular_table;
select count(*) > 0 from pg_visibility('regular_table');
?column?
----------
@@ -132,7 +132,7 @@ select count(*) > 0 from pg_visibility((select reltoastrelid from pg_class where
(1 row)
create materialized view matview_visibility_test as select * from regular_table;
-vacuum matview_visibility_test;+vacuum (disable_page_skipping) matview_visibility_test;
select count(*) > 0 from pg_visibility('matview_visibility_test');
?column?
----------
@@ -149,7 +149,7 @@ select count(*) > 0 from pg_visibility('matview_visibility_test');
-- regular tables which are part of a partition *do* have visibility maps
insert into test_partition values (1);
-vacuum test_partition;+vacuum (disable_page_skipping) test_partition;
select count(*) > 0 from pg_visibility('test_partition', 0);
?column?
----------
diff --git a/contrib/pg_visibility/sql/pg_visibility.sql b/contrib/pg_visibility/sql/pg_visibility.sqlindex ec1afd4906..ff3538f996 100644--- a/contrib/pg_visibility/sql/pg_visibility.sql+++ b/contrib/pg_visibility/sql/pg_visibility.sql@@ -71,7 +71,7 @@ select pg_truncate_visibility_map('test_foreign_table');
create table regular_table (a int, b text);
alter table regular_table alter column b set storage external;
insert into regular_table values (1, repeat('one', 1000)), (2, repeat('two', 1000));
-vacuum regular_table;+vacuum (disable_page_skipping) regular_table;
select count(*) > 0 from pg_visibility('regular_table');
select count(*) > 0 from pg_visibility((select reltoastrelid from pg_class where relname = 'regular_table'));
truncate regular_table;
@@ -79,7 +79,7 @@ select count(*) > 0 from pg_visibility('regular_table');
select count(*) > 0 from pg_visibility((select reltoastrelid from pg_class where relname = 'regular_table'));
create materialized view matview_visibility_test as select * from regular_table;
-vacuum matview_visibility_test;+vacuum (disable_page_skipping) matview_visibility_test;
select count(*) > 0 from pg_visibility('matview_visibility_test');
insert into regular_table values (1), (2);
refresh materialized view matview_visibility_test;
@@ -87,7 +87,7 @@ select count(*) > 0 from pg_visibility('matview_visibility_test');
-- regular tables which are part of a partition *do* have visibility maps
insert into test_partition values (1);
-vacuum test_partition;+vacuum (disable_page_skipping) test_partition;
select count(*) > 0 from pg_visibility('test_partition', 0);
select count(*) > 0 from pg_visibility_map('test_partition');
select count(*) > 0 from pg_visibility_map_summary('test_partition');
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: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Odd, intermittent failure in contrib/pageinspect
In-Reply-To: <550868.1611093829@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox