From 856bc0c612bea2df5f5748a51f2cda3f3c33928d Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 30 Apr 2019 17:45:32 -0700 Subject: Run catalog reindexing test from 3dbb317d32 serially, to avoid deadlocks. The tests turn out to cause deadlocks in some circumstances. Fairly reproducibly so with -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE. Some of the deadlocks may be hard to fix without disproportionate measures, but others probably should be fixed - but not in 12. We discussed removing the new tests until we can fix the issues underlying the deadlocks, but results from buildfarm animal markhor (which runs with CLOBBER_CACHE_ALWAYS) indicates that there might be a more severe, as of yet undiagnosed, issue (including on stable branches) with reindexing catalogs. The failure is: ERROR: could not read block 0 in file "base/16384/28025": read only 0 of 8192 bytes Therefore it seems advisable to keep the tests. It's not certain that running the tests in isolation removes the risk of deadlocks. It's possible that additional locks are needed to protect against a concurrent auto-analyze or such. Per discussion with Tom Lane. Discussion: https://postgr.es/m/28926.1556664156@sss.pgh.pa.us Backpatch: 9.4-, like 3dbb317d3 --- src/test/regress/expected/create_index.out | 18 -------------- src/test/regress/expected/reindex_catalog.out | 33 ++++++++++++++++++++++++ src/test/regress/parallel_schedule | 5 ++++ src/test/regress/serial_schedule | 1 + src/test/regress/sql/create_index.sql | 21 ---------------- src/test/regress/sql/reindex_catalog.sql | 36 +++++++++++++++++++++++++++ 6 files changed, 75 insertions(+), 39 deletions(-) create mode 100644 src/test/regress/expected/reindex_catalog.out create mode 100644 src/test/regress/sql/reindex_catalog.sql diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index d123d43d80..728cd1eabe 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2804,21 +2804,3 @@ explain (costs off) Index Cond: ((thousand = 1) AND (tenthous = 1001)) (2 rows) --- --- check that system tables can be reindexed --- --- whole tables -REINDEX TABLE pg_class; -- mapped, non-shared, critical -REINDEX TABLE pg_index; -- non-mapped, non-shared, critical -REINDEX TABLE pg_operator; -- non-mapped, non-shared, critical -REINDEX TABLE pg_database; -- mapped, shared, critical -REINDEX TABLE pg_shdescription; -- mapped, shared non-critical --- Check that individual system indexes can be reindexed. That's a bit --- different from the entire-table case because reindex_relation --- treats e.g. pg_class special. -REINDEX INDEX pg_class_oid_index; -- mapped, non-shared, critical -REINDEX INDEX pg_class_relname_nsp_index; -- mapped, non-shared, non-critical -REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical -REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical -REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical -REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical diff --git a/src/test/regress/expected/reindex_catalog.out b/src/test/regress/expected/reindex_catalog.out new file mode 100644 index 0000000000..142616fccb --- /dev/null +++ b/src/test/regress/expected/reindex_catalog.out @@ -0,0 +1,33 @@ +-- +-- Check that system tables can be reindexed. +-- +-- Note that this test currently has to run without parallel tests +-- being scheduled, as currently reindex catalog tables can cause +-- deadlocks: +-- +-- * The lock upgrade between the ShareLock acquired for the reindex +-- and RowExclusiveLock needed for pg_class/pg_index locks can +-- trigger deadlocks. +-- +-- * The uniqueness checks performed when reindexing a unique/primary +-- key index possibly need to wait for the transaction of a +-- about-to-deleted row in pg_class to commit. That can cause +-- deadlocks because, in contrast to user tables, locks on catalog +-- tables are routinely released before commit - therefore the lock +-- held for reindexing doesn't guarantee that no running transaction +-- performed modifications in the table underlying the index. +-- Check reindexing of whole tables +REINDEX TABLE pg_class; -- mapped, non-shared, critical +REINDEX TABLE pg_index; -- non-mapped, non-shared, critical +REINDEX TABLE pg_operator; -- non-mapped, non-shared, critical +REINDEX TABLE pg_database; -- mapped, shared, critical +REINDEX TABLE pg_shdescription; -- mapped, shared non-critical +-- Check that individual system indexes can be reindexed. That's a bit +-- different from the entire-table case because reindex_relation +-- treats e.g. pg_class special. +REINDEX INDEX pg_class_oid_index; -- mapped, non-shared, critical +REINDEX INDEX pg_class_relname_nsp_index; -- mapped, non-shared, non-critical +REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical +REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical +REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical +REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 8031489bd0..74d8973a91 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -56,6 +56,11 @@ test: create_misc create_operator # These depend on the above two test: create_index create_view +# ---------- +# Has to run in isolation, due to deadlock risk +# ---------- +test: reindex_catalog + # ---------- # Another group of parallel tests # ---------- diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule index a04dc91ffe..96fdba2222 100644 --- a/src/test/regress/serial_schedule +++ b/src/test/regress/serial_schedule @@ -60,6 +60,7 @@ test: create_misc test: create_operator test: create_index test: create_view +test: reindex_catalog test: create_aggregate test: create_function_3 test: create_cast diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index 0c6e386373..5008e981ab 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -951,24 +951,3 @@ RESET enable_indexonlyscan; explain (costs off) select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null)); - --- --- check that system tables can be reindexed --- - --- whole tables -REINDEX TABLE pg_class; -- mapped, non-shared, critical -REINDEX TABLE pg_index; -- non-mapped, non-shared, critical -REINDEX TABLE pg_operator; -- non-mapped, non-shared, critical -REINDEX TABLE pg_database; -- mapped, shared, critical -REINDEX TABLE pg_shdescription; -- mapped, shared non-critical - --- Check that individual system indexes can be reindexed. That's a bit --- different from the entire-table case because reindex_relation --- treats e.g. pg_class special. -REINDEX INDEX pg_class_oid_index; -- mapped, non-shared, critical -REINDEX INDEX pg_class_relname_nsp_index; -- mapped, non-shared, non-critical -REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical -REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical -REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical -REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical diff --git a/src/test/regress/sql/reindex_catalog.sql b/src/test/regress/sql/reindex_catalog.sql new file mode 100644 index 0000000000..2180ee5791 --- /dev/null +++ b/src/test/regress/sql/reindex_catalog.sql @@ -0,0 +1,36 @@ +-- +-- Check that system tables can be reindexed. +-- +-- Note that this test currently has to run without parallel tests +-- being scheduled, as currently reindex catalog tables can cause +-- deadlocks: +-- +-- * The lock upgrade between the ShareLock acquired for the reindex +-- and RowExclusiveLock needed for pg_class/pg_index locks can +-- trigger deadlocks. +-- +-- * The uniqueness checks performed when reindexing a unique/primary +-- key index possibly need to wait for the transaction of a +-- about-to-deleted row in pg_class to commit. That can cause +-- deadlocks because, in contrast to user tables, locks on catalog +-- tables are routinely released before commit - therefore the lock +-- held for reindexing doesn't guarantee that no running transaction +-- performed modifications in the table underlying the index. + + +-- Check reindexing of whole tables +REINDEX TABLE pg_class; -- mapped, non-shared, critical +REINDEX TABLE pg_index; -- non-mapped, non-shared, critical +REINDEX TABLE pg_operator; -- non-mapped, non-shared, critical +REINDEX TABLE pg_database; -- mapped, shared, critical +REINDEX TABLE pg_shdescription; -- mapped, shared non-critical + +-- Check that individual system indexes can be reindexed. That's a bit +-- different from the entire-table case because reindex_relation +-- treats e.g. pg_class special. +REINDEX INDEX pg_class_oid_index; -- mapped, non-shared, critical +REINDEX INDEX pg_class_relname_nsp_index; -- mapped, non-shared, non-critical +REINDEX INDEX pg_index_indexrelid_index; -- non-mapped, non-shared, critical +REINDEX INDEX pg_index_indrelid_index; -- non-mapped, non-shared, non-critical +REINDEX INDEX pg_database_oid_index; -- mapped, shared, critical +REINDEX INDEX pg_shdescription_o_c_index; -- mapped, shared, non-critical -- cgit v1.2.1