From: | Sergei Kornilov <sk(at)zsrv(dot)org> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: REINDEX CONCURRENTLY 2.0 |
Date: | 2019-03-23 19:04:47 |
Message-ID: | 1907751553367887@myt6-23299ba78d64.qloud-c.yandex.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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 change this 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 system catalog. Is this expected?
- should reindexdb check server version? For example, binary from patched HEAD can reindex v11 cluster and obviously fail if --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 get more attention from other committers?
regards, Sergei
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-03-23 20:59:04 | Re: Fix XML handling with DOCTYPE |
Previous Message | Fabien COELHO | 2019-03-23 18:45:33 | Re: CPU costs of random_zipfian in pgbench |