On Wed, Oct 3, 2012 at 5:10 PM, Andres Freund <and...@2ndquadrant.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