Re: pg_upgrade problem with invalid indexes

On Fri, Dec 7, 2012 at 10:29:22AM -0500, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > On Fri, Dec 7, 2012 at 03:32:51PM +0100, Andres Freund wrote:
> >> ISTM that anybody who does DDL during or after pg_upgrade --check
> >> deserves any pain.
> >>
> >> So throwing an error in both seems perfectly fine for me.
>
> I agree with Andres on this.
>
> > I am just saying that this makes the --check report more likely to false
> > failures than currently configured.
>
> It's not a "false" failure. If you were to force-shutdown the system at
> that instant (not allowing C.I.C. to complete) then do a pg_upgrade, it
> would fail. So what's wrong with telling the user so?

True.

> If you want, you could have the error message include some weasel
> wording about how this might be only a transient state, but frankly
> I think that's more likely to be confusing than helpful. As Andres
> says, nobody should expect that it's sensible to do "pg_upgrade --check"
> while any DDL is actively executing, so you'd be warning about a state
> that more than likely isn't reality anyway.

Well, if we fail, we want to give the user a solution to fixing it, and
that solution is going to be different if the index is in-process, or
from long ago. Those index names are going to be dumped into a file for
the user to review.

It will require weasel wording because pg_upgrade will have to say that
the indexes are either in the process of being concurrently created, or
are the result of a failed concurrent index creation in the past. They
will either have to wait for the index creation to complete or delete
the invalid index.

> On balance I am coming around to support the "just throw an error if
> there are any invalid indexes" position. Adding extra complication in
> pg_dump and pg_upgrade to handle ignoring them doesn't seem like a good
> idea --- for one thing, it will evidently weaken the strength of the
> same-number-of-relations cross-check.

The check would remain the same --- the change would be to prevent
invalid indexes from being considered on both the old and new servers.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +