On 2012-11-27 12:02:09 -0500, Tom Lane wrote: > Andres Freund <and...@2ndquadrant.com> writes: > > On 2012-11-26 16:39:39 -0500, Tom Lane wrote: > >> I looked at this a bit. I am very unhappy with the proposed kluge to > >> open indexes NoLock in some places. Even if that's safe today, which > >> I don't believe in the least, any future change in this area could break > >> it. > > > I am not happy with it either. But I failed to see a better way. Note > > that in most of the cases a lock actually is acquired shortly > > afterwards, just a ->indisvalid or an ->indisready check away. > > I think you're missing the point.
True. > The proposed patch will result in > RelationGetIndexAttrBitmap trying to do index_open() on an index that > might be getting dropped *right now* by a concurrent DROP INDEX > CONCURRENTLY. If the DROP commits while index_open is running, its > SnapshotNow catalog searches will stop finding relevant rows. From > the point of view of index_open, the relation data structure is then > corrupt (for example, it might not find pg_attribute entries for all > the columns) and so it will throw an error. > We need to fix things so that the final pre-deletion state of the > pg_index row tells observer backends not to even try to open the index. > (Thus, it will not matter whether they come across the pg_index row just > before or just after the deleting transaction commits --- either way, > they don't try to touch the index.) So that has to be a different state > from the initial state used during CREATE INDEX CONCURRENTLY. > > The point of not wanting to open the index NoLock (and for that matter > of having DROP INDEX CONCURRENTLY take AccessExclusiveLock once it > thinks nobody is touching the index) is to make sure that if somehow > somebody is touching the index anyway, they don't see the index's > catalog entries as corrupt. They'll either all be there or all not. > It's only a belt-and-suspenders safety measure, but I don't want to > give it up. So the idea would be to skip about-to-be-dropped indexes in RelationGetIndexList directly - we don't ever need to watch those, not even for HOT - because we only have the necessary knowledge there. The normal valid/ready checks will be done at the callsites of RelationGetIndexList(). But those checks can be done in a locked manner now. Are you already working on a fix? Or shall I work on an updated patch? I vote for introducing wrapper functions/macro to do the about-to-be-dropped check, its hard enough to understand as-is. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
