Re: Support for REINDEX CONCURRENTLY

Michael Paquier <michael.paquier@gmail.com>

From: Michael Paquier <michael.paquier@gmail.com>
To: Fujii Masao <masao.fujii@gmail.com>
Cc: Andres Freund <andres@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Peter Eisentraut <peter_e@gmx.net>, Tom Lane <tgl@sss.pgh.pa.us>, Simon Riggs <simon@2ndquadrant.com>
Date: 2013-03-06T20:26:31Z
Lists: pgsql-hackers

Attachments

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

Other opinions welcome.
-- 
Michael

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Background worker processes

  2. Fix assorted bugs in CREATE/DROP INDEX CONCURRENTLY.

  3. Work around unportable behavior of malloc(0) and realloc(NULL, 0).

  4. Properly set relpersistence for fake relcache entries.