Andres Freund <andres@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. 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.
regards, tom lane