On Thu, Mar 7, 2013 at 7:19 AM, Andres Freund <and...@2ndquadrant.com>wrote:
> On 2013-03-07 05:26:31 +0900, Michael Paquier wrote: > > On Thu, Mar 7, 2013 at 2:34 AM, Fujii Masao <masao.fu...@gmail.com> > wrote: > > > > > On Thu, Mar 7, 2013 at 2:17 AM, Andres Freund <and...@2ndquadrant.com> > > > wrote: > > > >> Indexes: > > > >> "hoge_pkey" PRIMARY KEY, btree (i) > > > >> "hoge_pkey_cct" PRIMARY KEY, btree (i) INVALID > > > >> "hoge_pkey_cct1" PRIMARY KEY, btree (i) INVALID > > > >> "hoge_pkey_cct_cct" PRIMARY KEY, btree (i) > > > > > > > > Huh, why did that go through? It should have errored out? > > > > > > I'm not sure why. Anyway hoge_pkey_cct_cct should not appear or should > > > be marked as invalid, I think. > > > > > CHECK_FOR_INTERRUPTS were not added at each phase and they are needed in > > case process is interrupted by user. This has been mentioned in a pas > > review but it was missing, so it might have slipped out during a > > refactoring or smth. Btw, I am surprised to see that this *_cct_cct index > > has been created knowing that hoge_pkey_cct is invalid. I tried with the > > latest version of the patch and even the patch attached but couldn't > > reproduce it. > > The strange think about "hoge_pkey_cct_cct" is that it seems to imply > that an invalid index was reindexed concurrently? > > But I don't see how it could happen either. Fujii, can you reproduce it? > Curious about that also. > > >> + The recommended recovery method in such cases is to drop the > > > concurrent > > > >> + index and try again to perform <command>REINDEX > CONCURRENTLY</>. > > > >> > > > >> If an invalid index depends on the constraint like primary key, > "drop > > > >> the concurrent > > > >> index" cannot actually drop the index. In this case, you need to > issue > > > >> "alter table > > > >> ... drop constraint ..." to recover the situation. I think this > > > >> informataion should be > > > >> documented. > > > > > > > > I think we just shouldn't set ->isprimary on the temporary indexes. > Now > > > > we switch only the relfilenodes and not the whole index, that should > be > > > > perfectly fine. > > > > > > Sounds good. But, what about other constraint case like unique > constraint? > > > Those other cases also can be resolved by not setting ->isprimary? > > > > > We should stick with the concurrent index being a twin of the index it > > rebuilds for consistency. > > I don't think its legal. We cannot simply have two indexes with > 'indisprimary'. Especially not if bot are valid. > Also, there will be no pg_constraint row that refers to it which > violates very valid expectations that both users and pg may have. > So what to do with that? Mark the concurrent index as valid, then validate it and finally mark it as invalid inside the same transaction at phase 4? That's moving 2 lines of code... -- Michael