Re: Heap lock levels for REINDEX INDEX CONCURRENTLY not quite right?

Hi,

On 2019-05-07 12:25:43 +0900, Michael Paquier wrote:
> On Tue, May 07, 2019 at 12:07:56PM +0900, Michael Paquier wrote:
> > Now, what do we do about the potential deadlock issues in
> > WaitForOlderSnapshots? The attached is an isolation test able to
> > reproduce the deadlock within WaitForOlderSnapshots() with two
> > parallel REINDEX CONCURRENTLY. I'd like to think that the best way to
> > do that would be to track in vacuumFlags the backends running a
> > REINDEX and just exclude them from GetCurrentVirtualXIDs() because
> > we don't actually care about missing index entries in this case like
> > VACUUM. But it looks also to me that is issue is broader and goes
> > down to utility commands which can take a lock on a table which cannot
> > be run in transaction blocks, hence code paths used by CREATE INDEX
> > CONCURRENTLY and DROP INDEX CONCURRENTLY could also cause a similar
> > deadlock, no?
>
> More to the point, one can just do that without REINDEX:
> - session 1:
> create table aa (a int);
> begin;
> lock aa in row exclusive mode;
> - session 2:
> create index concurrently aai on aa(a); --blocks
> - session 3:
> create index concurrently aai2 on aa(a); --blocks
> - session 1:
> commit;
>
> Then session 2 deadlocks while session 3 finishes correctly. I don't
> know if this is a class of problems we'd want to address for v12, but
> if we do then CIC (and DROP INDEX CONCURRENTLY?) could benefit from
> it.

This seems like a pre-existing issue to me. We probably should improve
that, but I don't think it has to be tied to 12.

Greetings,

Andres Freund