On Thu, Mar 7, 2013 at 9:48 AM, Michael Paquier <michael.paqu...@gmail.com>wrote:
> > > 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... > Sorry phase 4 is the swap phase. Validation happens at phase 3. -- Michael
