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

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 67893061f026b9ff393f124a7543332663268808..064adb4640bc3c99b7f73df7ae696192bfc86f8a 100644 (file)
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2998,24 +2998,6 @@ REINDEX (VERBOSE) TABLE reindex_verbose;
 INFO:  index "reindex_verbose_pkey" was reindexed
 DROP TABLE reindex_verbose;
 --
--- 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
---
 -- REINDEX SCHEMA
 --
 REINDEX SCHEMA schema_to_reindex; -- failure, schema does not exist

diff --git a/src/test/regress/expected/reindex_catalog.out b/src/test/regress/expected/reindex_catalog.out
new file mode 100644 (file)
index 0000000..142616f
--- /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 eefdeeacaef6fef26265705c33c4c81c1d04a260..e94e5b0b0e4635857ac486f24f209dcb3e50d958 100644 (file)
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -57,6 +57,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 76b0de30a7a515e91758e8459b8fd288702230e1..e062c52fd1e904b7329858e0e7028e0b76bdaee9 100644 (file)
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -65,6 +65,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 dbb116b72c62319665a22bb9e9482f38e6b72f41..67470db918b7bd6884c1363edd48551270106ce7 100644 (file)
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1033,27 +1033,6 @@ CREATE TABLE reindex_verbose(id integer primary key);
 REINDEX (VERBOSE) TABLE reindex_verbose;
 DROP TABLE reindex_verbose;
 
---
--- 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
-
 --
 -- REINDEX SCHEMA
 --

diff --git a/src/test/regress/sql/reindex_catalog.sql b/src/test/regress/sql/reindex_catalog.sql
new file mode 100644 (file)
index 0000000..2180ee5
--- /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