On Fri, Sep 11, 2015 at 11:29 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Wed, Sep 9, 2015 at 3:00 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> Some stuff Salesforce has been doing turned up the fact that >> RelationInitIndexAccessInfo is not safe against a relcache flush on >> its target index. Specifically, the failure goes like this: >> >> * RelationInitIndexAccessInfo loads up relation->rd_indextuple. >> >> * Within one of the subsequent catalog fetches, eg the pg_am read, a >> relcache flush occurs (this can be forced with CLOBBER_CACHE_ALWAYS). >> >> * RelationClearRelation keeps the relcache entry, since it has >> positive refcount, but doesn't see a reason to keep the index-related >> fields. >> >> * RelationInitIndexAccessInfo dumps core when it tries to fetch >> fields out of relation->rd_indextuple, which has been reset to NULL. >> >> Now, it turns out this scenario cannot happen in the standard Postgres >> code as it stands today (if it could, we'd have seen it in the buildfarm's >> CLOBBER_CACHE_ALWAYS testing). The reason for that is that >> RelationInitIndexAccessInfo is actually only called on newly-minted >> Relation structs that are not yet installed into the relcache hash table; >> hence RelationCacheInvalidate can't find them to zap them. It can happen >> in Salesforce's modified code though, and it's not hard to imagine that >> future changes in the community version might expose the same hazard. >> (For one reason, the current technique implies that an error halfway >> through relcache load results in leaking the Relation struct and >> subsidiary data; we might eventually decide that's not acceptable.) >> >> We could just ignore the problem until/unless it becomes real for us, >> but now that I've figured it out I'm inclined to do something before >> the knowledge disappears again. >> >> The specific reason there's a problem is that there's a disconnect between >> RelationClearRelation's test for whether a relcache entry has valid index >> info (it checks whether rd_indexcxt != NULL) and the order of operations >> in RelationInitIndexAccessInfo (creating the index context is not the >> first thing it does). Given that if RelationClearRelation thinks the >> info is valid, what it does is call RelationReloadIndexInfo which needs >> rd_index to be valid, I'm thinking the best fix is to leave the order of >> operations in RelationInitIndexAccessInfo alone and instead make the >> "relcache entry has index info" check be "rd_index != NULL". There might >> need to be some additional work to keep RelationReloadIndexInfo from >> setting rd_isvalid = true too soon. >> >> Thoughts? > > Doesn't seem like a bad thing to clean up.
Doesn't sound bad to me either to reinforce things on HEAD... It's been months since this has been posted, are you working on a patch? -- Michael -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers