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