Re: Support for REINDEX CONCURRENTLY

On Thu, Mar 7, 2013 at 9:48 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com>wrote:

>
>
> On Thu, Mar 7, 2013 at 7:19 AM, Andres Freund <andres(at)2ndquadrant(dot)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(dot)fujii(at)gmail(dot)com>
>> wrote:
>> >
>> > > On Thu, Mar 7, 2013 at 2:17 AM, Andres Freund <andres(at)2ndquadrant(dot)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