Re: Support for REINDEX CONCURRENTLY

On 2013-06-22 12:50:52 +0900, Michael Paquier wrote:
> On Fri, Jun 21, 2013 at 10:47 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > Hm. Looking at how this is currently used - I am afraid it's not
> > correct... the reason RelationGetIndexList() returns a copy is that
> > cache invalidations will throw away that list. And you do index_open()
> > while iterating over it which will accept invalidation messages.
> > Mybe it's better to try using RelationGetIndexList directly and measure
> > whether that has a measurable impact=
> By looking at the comments of RelationGetIndexList:relcache.c,
> actually the method of the patch is correct because in the event of a
> shared cache invalidation, rd_indexvalid is set to 0 when the index
> list is reset, so the index list would get recomputed even in the case
> of shared mem invalidation.

The problem I see is something else. Consider code like the following:

RelationFetchIndexListIfInvalid(toastrel);
foreach(lc, toastrel->rd_indexlist)
toastidxs[i++] = index_open(lfirst_oid(lc), RowExclusiveLock);

index_open calls relation_open calls LockRelationOid which does:
if (res != LOCKACQUIRE_ALREADY_HELD)
AcceptInvalidationMessages();

So, what might happen is that you open the first index, which accepts an
invalidation message which in turn might delete the indexlist. Which
means we would likely read invalid memory if there are two indexes.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services