Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l1z6U-0007sV-Mu for pgsql-hackers@arkaria.postgresql.org; Tue, 19 Jan 2021 22:03:58 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1l1z6T-0005Iz-Fw for pgsql-hackers@arkaria.postgresql.org; Tue, 19 Jan 2021 22:03:57 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l1z6T-0005Is-7k for pgsql-hackers@lists.postgresql.org; Tue, 19 Jan 2021 22:03:57 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1l1z6P-0001xP-BK for pgsql-hackers@lists.postgresql.org; Tue, 19 Jan 2021 22:03:56 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 10JM3naH550869; Tue, 19 Jan 2021 17:03:49 -0500 From: Tom Lane To: Michael Paquier cc: Alvaro Herrera , pgsql-hackers@lists.postgresql.org Subject: Re: Odd, intermittent failure in contrib/pageinspect In-reply-to: References: <20210118224005.GA3146@alvherre.pgsql> <416876.1611010060@sss.pgh.pa.us> Comments: In-reply-to Michael Paquier message dated "Tue, 19 Jan 2021 14:15:46 +0900" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <550844.1611093790.0@sss.pgh.pa.us> Date: Tue, 19 Jan 2021 17:03:49 -0500 Message-ID: <550868.1611093829@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <550844.1611093790.1@sss.pgh.pa.us> Michael Paquier 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="prevent-vacuum-page-skips-where-important.patch"; charset="us-ascii" Content-ID: <550844.1611093790.2@sss.pgh.pa.us> Content-Description: prevent-vacuum-page-skips-where-important.patch Content-Transfer-Encoding: quoted-printable diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/exp= ected/check_heap.out index 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 :=3D 'heaptest', startblock :=3D 100= 00, endblock :=3D 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 :=3D 'heaptest', skip :=3D 'none'); diff --git a/contrib/amcheck/sql/check_heap.sql b/contrib/amcheck/sql/chec= k_heap.sql index 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 :=3D 'heaptest', st= artblock :=3D 0, endblock :=3D SELECT * FROM verify_heapam(relation :=3D 'heaptest', startblock :=3D 100= 00, endblock :=3D 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/00= 1_verify_heapam.pl index 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) tes= t)); 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/e= xpected/page.out index 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_X= MAX_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/pa= ge.sql index 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_in= fomask, 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.out index 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, materiali= zed 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('tw= o', 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 reltoas= trelid 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_visibi= lity_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_visi= bility/sql/pg_visibility.sql index 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('tw= o', 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_clas= s where relname =3D '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_clas= s where relname =3D '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_visibili= ty_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'); ------- =_aaaaaaaaaa0--