Re: [HACKERS] REINDEX CONCURRENTLY 2.0

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Sergei Kornilov <sk(at)zsrv(dot)org>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Andreas Karlsson <andreas(at)proxel(dot)se>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Subject: Re: [HACKERS] REINDEX CONCURRENTLY 2.0
Date: 2019-01-03 21:49:55
Message-ID: f57aa1f7-cc66-7fdb-27a8-7eedbd1467c7@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Updated patch for some merge conflicts and addressing most of your
comments. (I did not do anything about the syntax.)

On 09/12/2018 19:55, Sergei Kornilov wrote:
> I found one error in phase 4. Simple reproducer:
>
>> create table test (i int);
>> create index this_is_very_large_exactly_maxnamelen_index_name_wink_wink_wink on test (i);
>> create index this_is_very_large_exactly_maxnamelen_index_name_wink_winkccold on test (i);
>> reindex table CONCURRENTLY test;
>
> This fails with error
>
>> ERROR: duplicate key value violates unique constraint "pg_class_relname_nsp_index"
>> DETAIL: Key (relname, relnamespace)=(this_is_very_large_exactly_maxnamelen_index_name_wink_win_ccold, 2200) already exists.
>
> CommandCounterIncrement() in (or after) index_concurrently_swap will fix this issue.

fixed

>> ReindexPartitionedIndex(Relation parentIdx)
>> ereport(ERROR,
>> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>> errmsg("REINDEX is not yet implemented for partitioned indexes")));
>
> I think we need add errhint("you can REINDEX each partition separately") or something similar.
> Also can we omit this warning for reindex database? All partition must be in same database and warning in such case is useless: we have warning, but doing reindex for each partition => we reindex partitioned table correctly.

fixed by skipping in ReindexRelationConcurrently(), same as other
unsupported relkinds

> Another behavior issue i found with reindex (verbose) schema/database: INFO ereport is printed twice for each table.
>
>> INFO: relation "measurement_y2006m02" was reindexed
>> DETAIL: CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.07 s.
>> INFO: table "public.measurement_y2006m02" was reindexed
>
> One from ReindexMultipleTables and another (with pg_rusage_show) from ReindexRelationConcurrently.

fixed with some restructuring

>> ReindexRelationConcurrently
>> if (!indexRelation->rd_index->indisvalid)
>
> it is better use IndexIsValid macro here? And same question about added indexform->indisvalid in src/backend/commands/tablecmds.c

IndexIsValid() has been removed in the meantime.

>> <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. To build the
>> - index without interfering with production you should drop the index and
>> - reissue the <command>CREATE INDEX CONCURRENTLY</command> command.
>> + convenient to use <command>REINDEX</command> to rebuild them.
>> </para>
>
> This documentation change seems wrong for me: reindex concurrently does not rebuild invalid indexes. To fix invalid indexes we still need reindex with lock table or recreate this index concurrently.

still being discussed elsewhere in this thread

>> + A first pass to build the index is done for each new index entry.
>> + Once the index is built, its flag <literal>pg_class.isready</literal> is
>> + switched to <quote>true</quote>
>> + At this point <literal>pg_class.indisvalid</literal> is switched to
>> + <quote>true</quote> for the new index and to <quote>false</quote> for the old, and
>> + Old indexes have <literal>pg_class.isready</literal> switched to <quote>false</quote>
>
> Should be pg_index.indisvalid and pg_index.indisready, right?

fixed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v6-0001-REINDEX-CONCURRENTLY.patch text/plain 90.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2019-01-03 21:50:24 Re: Delay locking partitions during query execution
Previous Message Hugh Ranalli 2019-01-03 21:48:33 Re: BUG #15548: Unaccent does not remove combining diacritical characters