Re: Support for REINDEX CONCURRENTLY

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Support for REINDEX CONCURRENTLY
Date: 2013-03-06 20:26:31
Message-ID: CAB7nPqQ4ze=-XdwDJ-Da=CaqYsSOxKgRAC2cz_Xn5UcddEyGcw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
20130306_1_remove_reltoastidxid_v4.patch application/octet-stream 39.1 KB
20130307_2_reindex_concurrently_v20.patch application/octet-stream 76.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2013-03-06 21:51:20 Re: Materialized views WIP patch
Previous Message Alvaro Herrera 2013-03-06 19:57:42 Re: Bug in tm2timestamp