Re: Support for REINDEX CONCURRENTLY

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?

> >> + 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.

> Also, I think that it is important from the session viewpoint to perform a
> swap with 2 valid indexes. If the process fails just before swapping
> indexes user might want to do that himself and drop the old index, then use
> the concurrent one.

The most likely outcome will be to rerun REINDEX CONCURRENTLY. Which
will then reindex one more index since it now has the old valid index
and the new valid index. Also, I don't think its fair game to expose
indexes that used to belong to a constraint without a constraint
supporting it as valid indexes.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services