Re: Support for REINDEX CONCURRENTLY

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