From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Sergei Kornilov <sk@zsrv.org>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Subject: Re: REINDEX CONCURRENTLY 2.0
Date: Fri, 12 Apr 2019 12:10:46 +0900
Message-ID: <20190412031046.GD2144@paquier.xyz> (raw)
In-Reply-To: <20190411134947.GA22043@alvherre.pgsql>
References: <20190411132704.GC30766@paquier.xyz>
<20190411134947.GA22043@alvherre.pgsql>
On Thu, Apr 11, 2019 at 09:49:47AM -0400, Alvaro Herrera wrote:
> Hmm, I suppose that makes sense for REINDEX TABLE or anything that
> reindexes more than one index, but if you do REINDEX INDEX surely it
> is reasonable to allow the case?
Yes, we could revisit the REINDEX INDEX portion of the decision, and
after sleeping on it my previous argument makes limited sense for
REINDEX processes using only one index. One could note that the
header comment of ReindexRelationConcurrently() kind of implies the
same conclusion as you do, but that's perhaps just an accident.
So... I am coming up with the patch attached. I have introduced some
tests using a trick with CIC to have an invalid index to work on.
--
Michael
Attachments:
[text/x-diff] reindex-conc-invalid.patch (6.5K, ../20190412031046.GD2144@paquier.xyz/2-reindex-conc-invalid.patch)
download | inline diff:diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgmlindex d9d95b20e3..929a326ae7 100644--- a/doc/src/sgml/ref/create_index.sgml+++ b/doc/src/sgml/ref/create_index.sgml@@ -555,10 +555,8 @@ Indexes:
The recommended recovery
method in such cases is to drop the index and try again to perform
- <command>CREATE INDEX CONCURRENTLY</command>. (Another possibility is to rebuild- the index with <command>REINDEX</command>. However, since <command>REINDEX</command>- does not support concurrent builds, this option is unlikely to seem- attractive.)+ <command>CREATE INDEX CONCURRENTLY</command>. (Another possibility is+ to rebuild the index with <command>REINDEX INDEX CONCURRENTLY</command>.
</para>
<para>
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgmlindex 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
<listitem>
<para>
- An index build with the <literal>CONCURRENTLY</literal> option failed, leaving- an <quote>invalid</quote> index. Such indexes are useless but it can be- convenient to use <command>REINDEX</command> to rebuild them. Note that- <command>REINDEX</command> 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 <command>CREATE INDEX CONCURRENTLY</command> command.+ If an index build fails with the <literal>CONCURRENTLY</literal> option,+ this index is left as <quote>invalid</quote>. Such indexes are useless+ but it can be convenient to use <command>REINDEX</command> to rebuild+ them. Note that only <command>REINDEX INDEX</command> is able+ to perform a concurrent build on an invalid index.
</para>
</listitem>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 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.outindex f9b4768aee..8bfcf57d5a 100644--- a/src/test/regress/expected/create_index.out+++ b/src/test/regress/expected/create_index.out@@ -2033,6 +2033,38 @@ 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.+-- And 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.sqlindex 2f0e9a63e6..74242098e3 100644--- a/src/test/regress/sql/create_index.sql+++ b/src/test/regress/sql/create_index.sql@@ -806,6 +806,21 @@ 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);+-- And 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
--
[application/pgp-signature] signature.asc (832B, ../20190412031046.GD2144@paquier.xyz/3-signature.asc)
download
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: michael@paquier.xyz, alvherre@2ndquadrant.com, ilmari@ilmari.org, peter.eisentraut@2ndquadrant.com, sk@zsrv.org, pgsql-hackers@lists.postgresql.org
Subject: Re: REINDEX CONCURRENTLY 2.0
In-Reply-To: <20190412031046.GD2144@paquier.xyz>
* 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