Re: Index corruption with CREATE INDEX CONCURRENTLY

On 2017-02-05 21:49:57 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > I've not yet read the full thread, but I'm a bit confused so far. We
> > obviously can get changing information about indexes here, but isn't
> > that something we have to deal with anyway? If we guarantee that we
> > don't loose knowledge that there's a pending invalidation, why do we
> > have to retry?
>
> We don't *have to* retry; the core of the fix is to not enter stale data
> into the cache after we've already received an invalidation signal.

Right, the bug is that we do so without remembering that fact.

> The current caller can survive with stale data; or if that's not true,
> C.I.C. has got more fundamental problems than posited here.

Indeed.

> But it seems like a generally bad idea for relcache to return data
> that it knows (or at least has reason to believe) is stale.

I'm not convinced by this - this kind of staleness can only occur if
changes happen during the execution of the cache building. And the
information returned can be outdated by pretty much the moment you
return anyway. It's also how a number of the current caches
essentially already work.

> Also, even if you're okay with return-stale-data-but-don't-cache-it,
> I have little faith that invalidate_indexattr_v3.patch successfully
> implements that.

I sent a prototype clarifying what I mean. It's not really like
invalidate_indexattr_v3.patch

Btw, it seems RelationGetIndexList() avoids a similar issue onlye due to
either luck or a lot of undocumented assumptions. The only reason that
setting relation->rd_indexlist / rd_indexvalid at the end doesn't cause
a similar issue seems to be that no invalidations appear to be processed
after the systable_beginscan(). And that in turn seems to not process
relcache invals after RegisterSnapshot(GetCatalogSnapshot(relid)).
Phew, that seems a bit fragile.

Greetings,

Andres Freund