Correct predicate locking for DROP INDEX CONCURRENTLY.

kgrittn · kgrittn · commit 523ecaf40436 · 2012-10-21T17:26:32.000-05:00
For the non-concurrent case there is an AccessExclusiveLock lock on both the index and the heap at a time during which no other process is using either, before which the index is maintained and used for scans, and after which the index is no longer used or maintained. Predicate locks can safely be moved from the index to the related heap relation under the protection of these locks. This was done prior to the introductin of DROP INDEX CONCURRENTLY and continues to be done for non-concurrent index drops. For concurrent index drops, the predicate locks must be moved when there are no index scans in progress on that index and no more can subsequently start, and before heap inserts stop maintaining the index. As long as these conditions are guaranteed when the TransferPredicateLocksToHeapRelation() function is called, stronger locks are not needed for correctness. Kevin Grittner based on questions by Tom Lane in reviewing the DROP INDEX CONCURRENTLY patch and in cooperation with Andres Freund and Simon Riggs. Back-patch of commit 4c9d090
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
@@ -1322,6 +1322,18 @@ index_drop(Oid indexId, bool concurrent)
 	 * 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
 	 * AccessExclusiveLock 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)
@@ -1446,6 +1458,14 @@ index_drop(Oid indexId, bool concurrent)
 			old_lockholders++;
 		}
 
+		/*
+		 * 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
@@ -1516,12 +1536,8 @@ 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);
+	else
+		TransferPredicateLocksToHeapRelation(userIndexRelation);
 
 	/*
 	 * Schedule physical removal of the files
