Hello
Yet another review of this patch from me...
> 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
changethis part.
> + The following steps occur in a concurrent index build, each in a separate
> + transaction except when the new index definitions are created
>
> + All the constraints and foreign keys which refer to the index are swapped...
> + ... This step is done within a single transaction
> + for each temporary entry.
>
> + 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.
And few questions:
- reindexdb has concurrently flag logic even in reindex_system_catalogs, but "reindex concurrently" can not reindex
systemcatalog. Is this expected?
- should reindexdb check server version? For example, binary from patched HEAD can reindex v11 cluster and obviously
failif --concurrently was requested.
- psql/tab-complete.c vs old releases? Seems we need suggest CONCURRENTLY keyword only for releases with concurrently
support.
Well, i still have no new questions about backend logic. Maybe we need mark patch as "Ready for Committer" in order to
getmore attention from other committers?
regards, Sergei
