Re: Support for REINDEX CONCURRENTLY

On Wed, Oct 3, 2012 at 5:10 PM, Andres Freund <andres(at)2ndquadrant(dot)com>wrote:

> > This basically allows to perform read and write operations on a table
> whose
> > index(es) are reindexed at the same time. Pretty useful for a production
> > environment. The caveats of this feature is that it is slower than
> normal
> > reindex, and impacts other backends with the extra CPU, memory and IO it
> > uses to process. The implementation is based on something on the same
> ideas
> > as pg_reorg and on an idea of Andres.
>
>
> > The following restrictions are applied.
> > - REINDEX [ DATABASE | SYSTEM ] cannot be run concurrently.
> I would like to support something like REINDEX USER TABLES; or similar at
> some
> point, but that very well can be a second phase.

This is something out of scope for the time being honestly. Later? why
not...

> > - REINDEX CONCURRENTLY cannot run inside a transaction block.
>
> > - toast relations are reindexed non-concurrently when table reindex is
> done
> > and that this table has toast relations
> Why that restriction?
>
This is the state of the current version of the patch. And not what the
final version should do. I agree that toast relations should also be
reindexed concurrently as the others. Regarding this current restriction,
my point was just to get some feedback before digging deeper. I should have
told that though...

>
> > Here is a description of what happens when reorganizing an index
> > concurrently
> > (the beginning of the process is similar to CREATE INDEX CONCURRENTLY):
> > 1) creation of a new index based on the same columns and restrictions as
> > the index that is rebuilt (called here old index). This new index has as
> > name $OLDINDEX_cct. So only a suffix _cct is added. It is marked as
> > invalid and not ready.
> You probably should take a SHARE UPDATE EXCLUSIVE lock on the table at that
> point already, to prevent schema changes.
>
> > 8) Take a reference snapshot and validate the new indexes
> Hm. Unless you factor in corrupt indices, why should this be needed?
>
> > 14) Swap new and old indexes, consisting here in switching their names.
> I think switching based on their names is not going to work very well
> because
> indexes are referenced by oid at several places. Swapping
> pg_index.indexrelid
> or pg_class.relfilenode seems to be the better choice to me. We expect
> relfilenode changes for such commands, but not ::regclass oid changes.
>
OK, so you mean to create an index, then switch only the relfilenode. Why
not. This is largely doable. I think that what is important here is to
choose a way of doing an keep it until the end.

>
> Such a behaviour would at least be complicated for pg_depend and
> pg_constraint.
>
> > The following process might be reducible, but I would like that to be
> > decided depending on the community feedback and experience on such
> > concurrent features.
> > For the time being I took an approach that looks slower, but secured to
> my
> > mind with multiple waits (perhaps sometimes unnecessary?) and
> > subtransactions.
>
> > If during the process an error occurs, the table will finish with either
> > the old or new index as invalid. In this case the user will be in charge
> to
> > drop the invalid index himself.
> > The concurrent index can be easily identified with its suffix *_cct.
> I am not really happy about relying on some arbitrary naming here. That
> still
> can result in conflicts and such.
>
The concurrent names are generated automatically with a function in
indexcmds.c, the same way a pkey indexes. Let's imagine that the
reindex concurrently command is run twice after a failure. The second
concurrent index will not have *_cct as suffix but _cct1. However I am open
to more ideas here. What I feel about the concurrent index is that it needs
a pg_class entry, even if it is just temporary, and this entry needs a name.

> > This patch has required some refactorisation effort as I noticed that the
> > code of index for concurrent operations was not very generic. In order
> to do
> > that, I created some new functions in index.c called index_concurrent_*
> > which are used by CREATE INDEX and REINDEX in my patch. Some refactoring
> has
> > also been done regarding the> wait processes.
>
> > REINDEX TABLE and REINDEX INDEX follow the same code path
> > (ReindexConcurrentIndexes in indexcmds.c). The patch structure is
> relying a
> > maximum on the functions of index.c when creating, building and
> validating
> > concurrent index.
> I haven't looked at the patch yet, but I was pretty sure that you would
> need
> to do quite some refactoring to implement this and this looks like roughly
> the
> right direction...
>
Thanks for spending time on it.
--
Michael Paquier
http://michael.otacoo.com