Re: REINDEX CONCURRENTLY 2.0

On 2019-03-23 20:04, Sergei Kornilov wrote:
>> 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.
>
> Not sure we can just say "use REINDEX" since only non-concurrently reindex can rebuild such index. I propose to not change this part.

Yeah, I reverted that and adjusted the wording a bit.

>> + Old indexes have <literal>pg_index.indisready</literal> switched to <quote>false</quote>
>> + to prevent any new tuple insertions after waiting for running queries which
>> + may reference the old index to complete. This step is done within a single
>> + transaction for each temporary entry.
>
> According to the code index_concurrently_swap is called in loop inside one transaction for all processed indexes of table. Same for index_concurrently_set_dead and index_concurrently_drop calls. So this part of documentation seems incorrect.

I rewrote that whole procedure to make it a bit simpler.

> And few questions:
> - reindexdb has concurrently flag logic even in reindex_system_catalogs, but "reindex concurrently" can not reindex system catalog. Is this expected?

If support is ever added, then reindexdb supports it automatically. It
seems simpler to not have to repeat the same checks in two places.

> - should reindexdb check server version? For example, binary from patched HEAD can reindex v11 cluster and obviously fail if --concurrently was requested.

Added.

> - psql/tab-complete.c vs old releases? Seems we need suggest CONCURRENTLY keyword only for releases with concurrently support.

It seems we don't do version checks for tab completion of keywords.

> Well, i still have no new questions about backend logic. Maybe we need mark patch as "Ready for Committer" in order to get more attention from other committers?

Let's do it. :-)

I've gone over this patch a few more times. I've read all the
discussion since 2012 again and made sure all the issues were addressed.
I made particularly sure that during the refactoring nothing in CREATE
INDEX CONCURRENTLY and DROP INDEX CONCURRENTLY was inadvertently
changed. I checked all the steps again. I'm happy with it.

One more change I made was in the drop phase. I had to hack it up a bit
so that we can call index_drop() with a concurrent lock but not actually
doing the concurrent processing (that would be a bit recursive). The
previous patch was actually taking too strong a lock here.

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