Re: REINDEX INDEX results in a crash for an index of pg_class since 9.6

Hi,

On 2019-04-30 14:41:00 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2019-04-30 14:05:50 -0400, Tom Lane wrote:
> >> Possibly we could run them in a TAP test that configures a cluster
> >> with autovac disabled?
>
> > Hm. Would it be sufficient to instead move them to a non-concurrent
> > test group, and stick a BEGIN; LOCK pg_class, ....; COMMIT; around it?
>
> Doubt it. Maybe you could get away with it given that autovacuum and
> autoanalyze only do non-transactional updates to pg_class, but that
> seems like a pretty shaky assumption.

I was pondering that autovacuum shouldn't play a role because it ought
to never cause a DELETE_IN_PROGRESS, because it shouldn't effect the
OldestXmin horizon. But that reasoning, even if correct, doesn't hold
for analyze, which does (much to my chargrin), holds a full blown
snapshot.

> > This is a pretty finnicky area of the code, with obviously not enough
> > test coverage. I'm inclined to remove them from the back branches, and
> > try to get them working in master?
>
> I think trying to get this "working" is a v13 task now. We've obviously
> never tried to stress the case before, so you're neither fixing a
> regression nor fixing a new-in-v12 issue.

Well, the test *do* test that a previously existing all-branches bug
doesn't exist, no (albeit one just triggering an assert)? I'm not
talking about making this concurrency safe, just about whether it's
possible to somehow keep the tests.

Greetings,

Andres Freund