SERVER-28938 Invalidate all cursors on collections to be renamed

3 files changed, 33 insertions, 10 deletions

diff --git a/jstests/core/getmore_invalidated_cursors.js b/jstests/core/getmore_invalidated_cursors.js
index 9b93d069d61..2d0bc20952d 100644
--- a/jstests/core/getmore_invalidated_cursors.js
+++ b/jstests/core/getmore_invalidated_cursors.js
@@ -87,4 +87,24 @@
     assert.commandWorked(testDB.runCommand({killCursors: coll.getName(), cursors: [cursorId]}));
     assert.commandFailedWithCode(testDB.runCommand({getMore: cursorId, collection: coll.getName()}),
                                  ErrorCodes.CursorNotFound);
+
+    // Test that all cursors on collections to be renamed get invalidated. Note that we can't do
+    // renames on sharded collections.
+    if (!isShardedCollection) {
+        setupCollection();
+        const collRenamed = testDB.test_rename;
+        collRenamed.drop();
+        cursor = coll.find().batchSize(batchSize);
+        assert(cursor.hasNext(), "Expected more data from find call on " + coll.getName());
+        assert.commandWorked(testDB.adminCommand({
+            renameCollection: testDB.getName() + "." + coll.getName(),
+            to: testDB.getName() + "." + collRenamed.getName()
+        }));
+
+        // Ensure getMore fails with an appropriate error code and message.
+        error = assert.throws(() => cursor.itcount());
+        assert.eq(error.code, ErrorCodes.OperationFailed, tojson(error));
+        assert.neq(-1, error.message.indexOf('collection dropped'), error.message);
+    }
+
 }());
diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index d0641b202a4..7abf504cbcb 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -432,8 +432,6 @@ Status DatabaseImpl::dropCollectionEvenIfSystem(OperationContext* opCtx,
 
     audit::logDropCollection(&cc(), fullns.toString());
 
-    collection->getCursorManager()->invalidateAll(opCtx, true, "collection dropped");
-
     Top::get(opCtx->getClient()->getServiceContext()).collectionDropped(fullns.toString());
 
     auto uuid = collection->uuid();
@@ -495,14 +493,16 @@ Status DatabaseImpl::_finishDropCollection(OperationContext* opCtx,
 
     // We want to destroy the Collection object before telling the StorageEngine to destroy the
     // RecordStore.
-    _clearCollectionCache(opCtx, fullns.toString(), "collection dropped");
+    _clearCollectionCache(
+        opCtx, fullns.toString(), "collection dropped", /*collectionGoingAway*/ true);
 
     return _dbEntry->dropCollection(opCtx, fullns.toString());
 }
 
 void DatabaseImpl::_clearCollectionCache(OperationContext* opCtx,
                                          StringData fullns,
-                                         const std::string& reason) {
+                                         const std::string& reason,
+                                         bool collectionGoingAway) {
     verify(_name == nsToDatabaseSubstring(fullns));
     CollectionMap::const_iterator it = _collections.find(fullns.toString());
 
@@ -512,7 +512,7 @@ void DatabaseImpl::_clearCollectionCache(OperationContext* opCtx,
     // Takes ownership of the collection
     opCtx->recoveryUnit()->registerChange(new RemoveCollectionChange(this, it->second));
 
-    it->second->getCursorManager()->invalidateAll(opCtx, false, reason);
+    it->second->getCursorManager()->invalidateAll(opCtx, collectionGoingAway, reason);
     _collections.erase(it);
 }
 
@@ -565,11 +565,12 @@ Status DatabaseImpl::renameCollection(OperationContext* opCtx,
 
         while (ii.more()) {
             IndexDescriptor* desc = ii.next();
-            _clearCollectionCache(opCtx, desc->indexNamespace(), clearCacheReason);
+            _clearCollectionCache(
+                opCtx, desc->indexNamespace(), clearCacheReason, /*collectionGoingAway*/ true);
         }
 
-        _clearCollectionCache(opCtx, fromNS, clearCacheReason);
-        _clearCollectionCache(opCtx, toNS, clearCacheReason);
+        _clearCollectionCache(opCtx, fromNS, clearCacheReason, /*collectionGoingAway*/ true);
+        _clearCollectionCache(opCtx, toNS, clearCacheReason, /*collectionGoingAway*/ false);
 
         Top::get(opCtx->getClient()->getServiceContext()).collectionDropped(fromNS.toString());
     }
diff --git a/src/mongo/db/catalog/database_impl.h b/src/mongo/db/catalog/database_impl.h
index d6e92f93c8c..4050bda4584 100644
--- a/src/mongo/db/catalog/database_impl.h
+++ b/src/mongo/db/catalog/database_impl.h
@@ -238,11 +238,13 @@ private:
 
     /**
      * Deregisters and invalidates all cursors on collection 'fullns'.  Callers must specify
-     * 'reason' for why the cache is being cleared.
+     * 'reason' for why the cache is being cleared. If 'collectionGoingAway' is false,
+     * unpinned cursors will not be killed.
      */
     void _clearCollectionCache(OperationContext* opCtx,
                                StringData fullns,
-                               const std::string& reason);
+                               const std::string& reason,
+                               bool collectionGoingAway);
 
     /**
      * Completes a collection drop by removing all the indexes and removing the collection itself