Andres Freund wrote: > On Thursday, October 18, 2012 06:12:02 AM Kevin Grittner wrote: >> I'm having trouble seeing a way to make this work without >> rearranging the code for concurrent drop to get to a state where >> it has set indisvalid = false, made that visible to all processes, >> and ensured that all scans of the index are complete -- while >> indisready is still true. That is the point where >> TransferPredicateLocksToHeapRelation() could be safely called. >> Then we would need to set indisready = false, make that visible to >> all processes, and ensure that all access to the index is >> complete. I can't see where it works to set both flags at the same >> time.
> In a nearby bug I had to restructure the code that in a way thats > similar to this anyway, so that seems fine. Maybe you can fix the > bug ontop of the two attached patches? Perfect; these two patches provide a spot in the code which is exactly right for handling the predicate lock adjustments. Attached is a patch which applies on top of the two you sent. Thanks! -Kevin
*** a/src/backend/catalog/index.c --- b/src/backend/catalog/index.c *************** *** 1320,1325 **** index_drop(Oid indexId, bool concurrent) --- 1320,1337 ---- * In the concurrent case we make sure that nobody can be looking at the * indexes by dropping the index in multiple steps, so we don't need a full * fledged AccessExlusiveLock yet. + * + * All predicate locks on the index are about to be made invalid. Promote + * them to relation locks on the heap. For correctness the index must not + * be seen with indisvalid = true during query planning after the move + * starts, so that the index will not be used for a scan after the + * predicate lock move, as this could create new predicate locks on the + * index which would not ensure a heap relation lock. Also, the index must + * not be seen during execution of a heap tuple insert with indisready = + * false before the move is complete, since the conflict with the + * predicate lock on the index gap could be missed before the lock on the + * heap relation is in place to detect a conflict based on the heap tuple + * insert. */ heapId = IndexGetRelation(indexId, false); if (concurrent) *************** *** 1439,1444 **** index_drop(Oid indexId, bool concurrent) --- 1451,1464 ---- } /* + * No more predicate locks will be acquired on this index, and we're + * about to stop doing inserts into the index which could show + * conflicts with existing predicate locks, so now is the time to move + * them to the heap relation. + */ + TransferPredicateLocksToHeapRelation(userIndexRelation); + + /* * now we are sure that nobody uses the index for queries, they just * might have it opened for updating it. So now we can unset * ->indisready and wait till nobody could update the index anymore. *************** *** 1507,1518 **** index_drop(Oid indexId, bool concurrent) userHeapRelation = heap_open(heapId, ShareUpdateExclusiveLock); userIndexRelation = index_open(indexId, AccessExclusiveLock); } ! ! /* ! * All predicate locks on the index are about to be made invalid. Promote ! * them to relation locks on the heap. ! */ ! TransferPredicateLocksToHeapRelation(userIndexRelation); /* * Schedule physical removal of the files --- 1527,1534 ---- userHeapRelation = heap_open(heapId, ShareUpdateExclusiveLock); userIndexRelation = index_open(indexId, AccessExclusiveLock); } ! else ! TransferPredicateLocksToHeapRelation(userIndexRelation); /* * Schedule physical removal of the files
-- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
