On Sat, Jun 22, 2013 at 10:34 PM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2013-06-22 12:50:52 +0900, Michael Paquier wrote: >> On Fri, Jun 21, 2013 at 10:47 PM, Andres Freund <and...@2ndquadrant.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. And I imagine that you have the same problem even with RelationGetIndexList, not only RelationGetIndexListIfInvalid, because this would appear as long as you try to open more than 1 index with an index list. -- Michael
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
