From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Bugs in CREATE/DROP INDEX CONCURRENTLY |
Date: | 2012-11-27 19:50:41 |
Message-ID: | 20121127195041.GE22677@awork2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2012-11-27 14:41:34 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2012-11-27 13:45:08 -0500, Tom Lane wrote:
> >> In short, all flag changes in pg_index should be done by
> >> update-in-place, and the tuple's xmin will never change from the
> >> original creating transaction (until frozen).
>
> > Hm. That doesn't sound that easy to guarantee. Several ALTER TABLE and
> > ALTER INDEX operations are expected to work transactionally right
> > now. As far as I see there is nothing that prohibits a indexcheckxmin
> > requiring index to be altered while indexcheckxmin is still required?
>
> I said "in pg_index". There is no reason to ever alter an index's
> pg_index entry transactionally, because we don't support redefining
> the index columns. The stuff you are allowed to ALTER is pretty much
> irrelevant to the index's life as an index.
Isn't inisprimary updated when an ALTER TABLE ... ADD PRIMARY KEY
... USING someindex ; is done? Also I think indoption might be written
to as well.
> >> What we want the xmin to do, for indcheckxmin purposes, is reflect the
> >> time at which the index started to be included in HOT-safety decisions.
> >> Since we're never going to move the xmin, that means that *we cannot
> >> allow REINDEX to mark a dead index live again*.
>
> > That would be a regression compared with the current state though. We
> > have officially documented REINDEX as a way out of INVALID indexes...
>
> It's a way out of failed CREATE operations. If DROP fails at the last
> step, you won't be able to go back, but why would you want to? Just
> do the DROP again.
Oh, sorry, misunderstood you.
>
> >> Anybody feel like bikeshedding the flag column name? I'm thinking
> >> "indislive" but maybe there's a better name.
>
> > I personally would slightly favor indisdead instead...
>
> Meh ... the other two flags are positive, in the sense of
> true-is-the-normal-state, so I thought this one should be too.
Good point.
> I had also toyed with "indishot", to reflect the idea that this controls
> whether the index is included in HOT-safety decisions, but that seems
> maybe a little too cute.
indislive seems better than that, yes.
> >>> Btw, even if we manage to find a sensible fix for this I would suggest
> >>> postponing it after the next back branch release.
>
> >> AFAICS this is a data loss/corruption problem, and as such a "must fix".
> >> If we can't have it done by next week, I'd rather postpone the releases
> >> until it is done.
>
> > Ok, just seemed like a rather complex fix in a short time for something
> > that seemingly hasn't been noticed since 8.3. I am a bit worried about
> > introducing something worse while fixing this.
>
> Hm? The fact that the DROP patch broke CREATE INDEX CONCURRENTLY is a
> new and very nasty bug in 9.2. I would agree with you if we were
> considering the unsafe-row-update problem alone, but it seems like we
> might as well fix both aspects while we're looking at this code.
> There is a question of whether we should risk trying to back-patch the
> in-place-update changes further than 9.2. Given the lack of complaints
> I'm inclined not to, but could be persuaded differently.
Oh, I only was talking about the inplace changes. The DROP INDEX
CONCURRENTLY breakage definitely needs to get backpatched.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2012-11-27 19:54:03 | Re: splitting *_desc routines |
Previous Message | Pavel Stehule | 2012-11-27 19:45:01 | Re: plpgsql_check_function - rebase for 9.3 |