From a6dcf9df4d91ff0db23579f9114079abe6f3e2bf Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 17 Apr 2019 09:33:51 +0900 Subject: [PATCH] Rework handling of invalid indexes with REINDEX CONCURRENTLY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per discussion with others, allowing REINDEX INDEX CONCURRENTLY to work for invalid indexes when working directly on them can have a lot of value to unlock situations with invalid indexes without having to use a dance involving DROP INDEX followed by an extra CREATE INDEX CONCURRENTLY (which would not work for indexes with constraint dependency anyway). This also does not create extra bloat on the relation involved as this works on individual indexes, so let's enable it. Note that REINDEX TABLE CONCURRENTLY still bypasses invalid indexes as we don't want to bloat the number of indexes defined on a relation in the event of multiple and successive failures of REINDEX CONCURRENTLY. More regression tests are added to cover those behaviors, using an invalid index created with CREATE INDEX CONCURRENTLY. Reported-by: Dagfinn Ilmari Mannsåker, Álvaro Herrera Author: Michael Paquier Reviewed-by: Peter Eisentraut, Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/20190411134947.GA22043@alvherre.pgsql --- doc/src/sgml/ref/create_index.sgml | 6 +-- doc/src/sgml/ref/reindex.sgml | 11 +++-- src/backend/commands/indexcmds.c | 29 +++---------- src/test/regress/expected/create_index.out | 47 ++++++++++++++++++++++ src/test/regress/sql/create_index.sql | 20 +++++++++ 5 files changed, 80 insertions(+), 33 deletions(-) diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml index e62ab33905..521e32b197 100644 --- a/doc/src/sgml/ref/create_index.sgml +++ b/doc/src/sgml/ref/create_index.sgml @@ -586,10 +586,8 @@ Indexes: The recommended recovery method in such cases is to drop the index and try again to perform - CREATE INDEX CONCURRENTLY. (Another possibility is to rebuild - the index with REINDEX. However, since REINDEX - does not support concurrent builds, this option is unlikely to seem - attractive.) + CREATE INDEX CONCURRENTLY. (Another possibility is + to rebuild the index with REINDEX INDEX CONCURRENTLY). diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index e05a76c6d8..303436c89d 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -65,12 +65,11 @@ REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURR - An index build with the CONCURRENTLY option failed, leaving - an invalid index. Such indexes are useless but it can be - convenient to use REINDEX to rebuild them. Note that - REINDEX will not perform a concurrent build on an invalid index. To build the - index without interfering with production you should drop the index and - reissue the CREATE INDEX CONCURRENTLY command. + If an index build fails with the CONCURRENTLY option, + this index is left as invalid. Such indexes are useless + but it can be convenient to use REINDEX to rebuild + them. Note that only REINDEX INDEX is able + to perform a concurrent build on an invalid index. diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 46f32c21f9..a1c91b5fb8 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2776,11 +2776,6 @@ ReindexRelationConcurrently(Oid relationOid, int options) } case RELKIND_INDEX: { - /* - * For an index simply add its Oid to list. Invalid indexes - * cannot be included in list. - */ - Relation indexRelation = index_open(relationOid, ShareUpdateExclusiveLock); Oid heapId = IndexGetRelation(relationOid, false); /* A shared relation cannot be reindexed concurrently */ @@ -2801,25 +2796,13 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Track the heap relation of this index for session locks */ heapRelationIds = list_make1_oid(heapId); + /* + * Save the list of relation OIDs in private context. Note + * that invalid indexes are allowed here. + */ + indexIds = lappend_oid(indexIds, relationOid); + MemoryContextSwitchTo(oldcontext); - - if (!indexRelation->rd_index->indisvalid) - ereport(WARNING, - (errcode(ERRCODE_INDEX_CORRUPTED), - errmsg("cannot reindex concurrently invalid index \"%s.%s\", skipping", - get_namespace_name(get_rel_namespace(relationOid)), - get_rel_name(relationOid)))); - else - { - /* Save the list of relation OIDs in private context */ - oldcontext = MemoryContextSwitchTo(private_context); - - indexIds = lappend_oid(indexIds, relationOid); - - MemoryContextSwitchTo(oldcontext); - } - - index_close(indexRelation, NoLock); break; } case RELKIND_PARTITIONED_TABLE: diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 39159e0915..326dc44177 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2118,6 +2118,53 @@ Referenced by: DROP MATERIALIZED VIEW concur_reindex_matview; DROP TABLE concur_reindex_tab, concur_reindex_tab2, concur_reindex_tab3; +-- Check handling of invalid indexes +CREATE TABLE concur_reindex_tab4 (c1 int); +INSERT INTO concur_reindex_tab4 VALUES (1), (1), (2); +-- This trick creates an invalid index. +CREATE UNIQUE INDEX CONCURRENTLY concur_reindex_ind5 ON concur_reindex_tab4 (c1); +ERROR: could not create unique index "concur_reindex_ind5" +DETAIL: Key (c1)=(1) is duplicated. +-- Reindexing concurrently this index fails with the same failure. +-- The extra index created is itself invalid, and can be dropped. +REINDEX INDEX CONCURRENTLY concur_reindex_ind5; +ERROR: could not create unique index "concur_reindex_ind5_ccnew" +DETAIL: Key (c1)=(1) is duplicated. +\d concur_reindex_tab4 + Table "public.concur_reindex_tab4" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + c1 | integer | | | +Indexes: + "concur_reindex_ind5" UNIQUE, btree (c1) INVALID + "concur_reindex_ind5_ccnew" UNIQUE, btree (c1) INVALID + +DROP INDEX concur_reindex_ind5_ccnew; +-- This makes the previous failure go away, so the index can become valid. +DELETE FROM concur_reindex_tab4 WHERE c1 = 1; +-- The invalid index is not processed when running REINDEX TABLE. +REINDEX TABLE CONCURRENTLY concur_reindex_tab4; +WARNING: cannot reindex concurrently invalid index "public.concur_reindex_ind5", skipping +NOTICE: table "concur_reindex_tab4" has no indexes +\d concur_reindex_tab4 + Table "public.concur_reindex_tab4" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + c1 | integer | | | +Indexes: + "concur_reindex_ind5" UNIQUE, btree (c1) INVALID + +-- But it is fixed with REINDEX INDEX. +REINDEX INDEX CONCURRENTLY concur_reindex_ind5; +\d concur_reindex_tab4 + Table "public.concur_reindex_tab4" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + c1 | integer | | | +Indexes: + "concur_reindex_ind5" UNIQUE, btree (c1) + +DROP TABLE concur_reindex_tab4; -- -- REINDEX SCHEMA -- diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f8141c0ce5..f29b8ca826 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -849,6 +849,26 @@ REINDEX SCHEMA CONCURRENTLY pg_catalog; DROP MATERIALIZED VIEW concur_reindex_matview; DROP TABLE concur_reindex_tab, concur_reindex_tab2, concur_reindex_tab3; +-- Check handling of invalid indexes +CREATE TABLE concur_reindex_tab4 (c1 int); +INSERT INTO concur_reindex_tab4 VALUES (1), (1), (2); +-- This trick creates an invalid index. +CREATE UNIQUE INDEX CONCURRENTLY concur_reindex_ind5 ON concur_reindex_tab4 (c1); +-- Reindexing concurrently this index fails with the same failure. +-- The extra index created is itself invalid, and can be dropped. +REINDEX INDEX CONCURRENTLY concur_reindex_ind5; +\d concur_reindex_tab4 +DROP INDEX concur_reindex_ind5_ccnew; +-- This makes the previous failure go away, so the index can become valid. +DELETE FROM concur_reindex_tab4 WHERE c1 = 1; +-- The invalid index is not processed when running REINDEX TABLE. +REINDEX TABLE CONCURRENTLY concur_reindex_tab4; +\d concur_reindex_tab4 +-- But it is fixed with REINDEX INDEX. +REINDEX INDEX CONCURRENTLY concur_reindex_ind5; +\d concur_reindex_tab4 +DROP TABLE concur_reindex_tab4; + -- -- REINDEX SCHEMA --