SERVER-28022 Invalidate old IDs for moved system.namespaces records

(cherry picked from commit b21ff4b1a89bf90a3cceee6961c75f8d10c733e6)

 Conflicts:
	src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp
	src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h

7 files changed, 102 insertions, 64 deletions

diff --git a/jstests/core/list_indexes_invalidation.js b/jstests/core/list_indexes_invalidation.js
index 1a7770b75fb..b908c7de045 100644
--- a/jstests/core/list_indexes_invalidation.js
+++ b/jstests/core/list_indexes_invalidation.js
@@ -9,19 +9,22 @@
     function testIndexInvalidation(isRename) {
         coll.drop();
         collRenamed.drop();
-        assert.commandWorked(coll.createIndexes([{a: 1}, {b: 1}, {c: 1}]));
+        assert.commandWorked(coll.createIndex({a: 1}));
+        assert.commandWorked(coll.createIndex({b: 1}));
+        assert.commandWorked(coll.createIndex({c: 1}));
 
         // Get the first two indexes. Use find on 'system.indexes' on MMAPv1, listIndexes otherwise.
-        var cmd = db.system.indexes.count() ? {find: 'system.indexes'} : {listIndexes: collName};
-        Object.extend(cmd, {batchSize: 2});
-        var res = db.runCommand(cmd);
-        assert.commandWorked(res, 'could not run ' + tojson(cmd));
-        printjson(res);
+        var cursor;
+        if (db.system.indexes.count()) {
+            cursor = db.system.indexes.find().batchSize(2);
+        } else {
+            var res = db.runCommand({listIndexes: collName});
+            assert.commandWorked(res, 'could not run listIndexes');
+            cursor = new DBCommandCursor(db.getMongo(), res);
+        }
 
         // Ensure the cursor has data, rename or drop the collection, and exhaust the cursor.
-        var cursor = new DBCommandCursor(db.getMongo(), res);
-        var errMsg =
-            'expected more data from command ' + tojson(cmd) + ', with result ' + tojson(res);
+        var errMsg = 'expected more data from system.indexes find cursor';
         assert(cursor.hasNext(), errMsg);
         if (isRename) {
             assert.commandWorked(coll.renameCollection(collNameRenamed));
diff --git a/jstests/core/list_namespaces_invalidation.js b/jstests/core/list_namespaces_invalidation.js
index 81f6c7d69f1..bcf75ae0695 100644
--- a/jstests/core/list_namespaces_invalidation.js
+++ b/jstests/core/list_namespaces_invalidation.js
@@ -1,45 +1,49 @@
-// SERVER-27996 Missing invalidation for system.namespaces writes
+// SERVER-27996/SERVER-28022 Missing invalidation for system.namespaces writes
 (function() {
     'use strict';
-    let dbInvalidName = 'system_namespaces_invalidations';
-    let dbInvalid = db.getSiblingDB(dbInvalidName);
-    let num_collections = 3;
-    function testNamespaceInvalidation(isRename) {
+    var dbInvalidName = 'system_namespaces_invalidations';
+    var dbInvalid = db.getSiblingDB(dbInvalidName);
+    var num_collections = 3;
+    var DROP = 1;
+    var RENAME = 2;
+    function testNamespaceInvalidation(namespaceAction, batchSize) {
         dbInvalid.dropDatabase();
 
         // Create enough collections to necessitate multiple cursor batches.
-        for (let i = 0; i < num_collections; i++) {
+        for (var i = 0; i < num_collections; i++) {
             assert.commandWorked(dbInvalid.createCollection('coll' + i.toString()));
         }
 
-        // Get the first two namespaces. Use find on 'system.namespaces' on MMAPv1, listCollections
-        // otherwise.
-        let cmd = dbInvalid.system.indexes.count() ? {find: 'system.namespaces'}
-                                                   : {listCollections: dbInvalidName};
-        Object.extend(cmd, {batchSize: 2});
-        let res = dbInvalid.runCommand(cmd);
-        assert.commandWorked(res, 'could not run ' + tojson(cmd));
-        printjson(res);
+        // Get the first batch of namespaces. Use find on 'system.namespaces' on MMAPv1,
+        // listCollections otherwise.
+        var cursor;
+        if (dbInvalid.system.namespaces.count()) {
+            cursor = dbInvalid.system.namespaces.find().batchSize(batchSize);
+        } else {
+            var res = dbInvalid.runCommand({listCollections: dbInvalidName});
+            assert.commandWorked(res, 'could not run listCollections');
+            cursor = new DBCommandCursor(dbInvalid.getMongo(), res);
+        }
 
-        // Ensure the cursor has data, drop or rename the collections, and exhaust the cursor.
-        let cursor = new DBCommandCursor(dbInvalid.getMongo(), res);
-        let errMsg =
-            'expected more data from command ' + tojson(cmd) + ', with result ' + tojson(res);
+        // Ensure the cursor has data, invalidate the namespace, and exhaust the cursor.
+        var errMsg = 'expected more data from system.namespaces cursor';
         assert(cursor.hasNext(), errMsg);
-        for (let j = 0; j < num_collections; j++) {
-            if (isRename) {
-                // Rename the collection to something that does not fit in the previously allocated
-                // memory for the record.
-                assert.commandWorked(dbInvalid['coll' + j.toString()].renameCollection(
-                    'coll' + j.toString() + 'lkdsahflaksjdhfsdkljhfskladhfkahfsakfla' +
-                    'skfjhaslfaslfkhasklfjhsakljhdsjksahkldjslh'));
-            } else {
-                assert(dbInvalid['coll' + j.toString()].drop());
-            }
+        if (namespaceAction == RENAME) {
+            // Rename the collection to something that does not fit in the previously allocated
+            // memory for the record.
+            assert.commandWorked(
+                dbInvalid['coll1'].renameCollection('coll1' +
+                                                    'lkdsahflaksjdhfsdkljhfskladhfkahfsakfla' +
+                                                    'skfjhaslfaslfkhasklfjhsakljhdsjksahkldjslh'));
+        } else if (namespaceAction == DROP) {
+            assert(dbInvalid['coll1'].drop());
         }
         assert.gt(cursor.itcount(), 0, errMsg);
     }
-    // Test that we invalidate namespaces for both collection drops and renames.
-    testNamespaceInvalidation(false);
-    testNamespaceInvalidation(true);
+    // Test that we invalidate the old namespace record ID when we remove or rename a namespace
+    // record.
+    for (var j = 2; j < 7; j++) {
+        testNamespaceInvalidation(DROP, j);
+        testNamespaceInvalidation(RENAME, j);
+    }
 }());
diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp
index 533f0d428d0..928548101c7 100644
--- a/src/mongo/db/catalog/collection.cpp
+++ b/src/mongo/db/catalog/collection.cpp
@@ -632,4 +632,8 @@ Status Collection::touch(OperationContext* txn,
 
     return Status::OK();
 }
+
+UpdateNotifier* Collection::getUpdateNotifier() {
+    return this;
+}
 }
diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h
index d64dd95aef8..2b2fd73c065 100644
--- a/src/mongo/db/catalog/collection.h
+++ b/src/mongo/db/catalog/collection.h
@@ -296,6 +296,12 @@ public:
 
     // --- end suspect things
 
+    /**
+     * This function is necessary for a 3.2 and 3.0 backport. We have a better fix for the
+     * underlying issue in later versions.
+     */
+    UpdateNotifier* getUpdateNotifier();
+
 private:
     Status recordStoreGoingToMove(OperationContext* txn,
                                   const RecordId& oldLocation,
diff --git a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp
index d8b32fbe47c..092312a97b4 100644
--- a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp
+++ b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp
@@ -32,6 +32,9 @@
 
 #include "mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h"
 
+#include "mongo/db/catalog/collection.h"
+#include "mongo/db/catalog/database.h"
+#include "mongo/db/catalog/database_holder.h"
 #include "mongo/db/index/index_descriptor.h"
 #include "mongo/db/ops/update.h"
 #include "mongo/db/record_id.h"
@@ -252,6 +255,11 @@ Status NamespaceDetailsCollectionCatalogEntry::removeIndex(OperationContext* txn
         d->idx(getTotalIndexCount(txn)) = IndexDetails();
     }
 
+    // Someone may be querying the system.indexes namespace directly, so we need to invalidate
+    // its cursors.
+    MMAPV1DatabaseCatalogEntry::invalidateSystemCollectionRecord(
+        txn, NamespaceString(_db->name(), "system.indexes"), infoLocation);
+
     // remove from system.indexes
     _indexRecordStore->deleteRecord(txn, infoLocation);
 
@@ -361,8 +369,20 @@ void NamespaceDetailsCollectionCatalogEntry::_updateSystemNamespaces(OperationCo
 
     RecordData entry = _namespacesRecordStore->dataFor(txn, _namespacesRecordId);
     const BSONObj newEntry = applyUpdateOperators(entry.releaseToBson(), update);
-    StatusWith<RecordId> result = _namespacesRecordStore->updateRecord(
-        txn, _namespacesRecordId, newEntry.objdata(), newEntry.objsize(), false, NULL);
+
+    // Get update notifier
+    invariant(txn->lockState()->isDbLockedForMode(_db->name(), MODE_X));
+    Database* db = dbHolder().get(txn, _db->name());
+    Collection* systemCollection =
+        db->getCollection(NamespaceString(_db->name(), "system.namespaces"));
+    UpdateNotifier* namespacesNotifier = systemCollection->getUpdateNotifier();
+
+    StatusWith<RecordId> result = _namespacesRecordStore->updateRecord(txn,
+                                                                       _namespacesRecordId,
+                                                                       newEntry.objdata(),
+                                                                       newEntry.objsize(),
+                                                                       false,
+                                                                       namespacesNotifier);
     fassert(17486, result.getStatus());
     setNamespacesRecordId(txn, result.getValue());
 }
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp
index f6e0e6c185c..b46a632fd5c 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.cpp
@@ -311,12 +311,7 @@ Status MMAPV1DatabaseCatalogEntry::renameCollection(OperationContext* txn,
                 return s;
         }
         // Invalidate index record for the old collection.
-        StringData dbName(nsToDatabaseSubstring(_collections.begin()->first));
-        invariant(txn->lockState()->isDbLockedForMode(dbName, MODE_X));
-        Database* db = dbHolder().get(txn, dbName);
-        Collection* systemIndexes = db->getCollection(db->getSystemIndexesName());
-        systemIndexes->getCursorManager()->invalidateDocument(
-            txn, record->id, INVALIDATION_DELETION);
+        invalidateSystemCollectionRecord(txn, NamespaceString(name(), "system.indexes"), loc);
 
         systemIndexRecordStore->deleteRecord(txn, loc);
     }
@@ -384,13 +379,8 @@ Status MMAPV1DatabaseCatalogEntry::_renameSingleNamespace(OperationContext* txn,
     RecordId rid = _addNamespaceToNamespaceCollection(txn, toNS, newSpec.isEmpty() ? 0 : &newSpec);
 
     // Invalidate old namespace record
-    const NamespaceString nsn(name(), "system.namespaces");
-    StringData dbName(name());
-    invariant(txn->lockState()->isDbLockedForMode(dbName, MODE_X));
-    Database* db = dbHolder().get(txn, dbName);
-    Collection* systemNamespaces = db->getCollection(nsn);
-    systemNamespaces->getCursorManager()->invalidateDocument(
-        txn, oldSpecLocation, INVALIDATION_DELETION);
+    invalidateSystemCollectionRecord(
+        txn, NamespaceString(name(), "system.namespaces"), oldSpecLocation);
 
     _getNamespaceRecordStore()->deleteRecord(txn, oldSpecLocation);
 
@@ -404,6 +394,17 @@ Status MMAPV1DatabaseCatalogEntry::_renameSingleNamespace(OperationContext* txn,
     return Status::OK();
 }
 
+void MMAPV1DatabaseCatalogEntry::invalidateSystemCollectionRecord(
+    OperationContext* txn, NamespaceString systemCollectionNamespace, RecordId record) {
+    // Having to go back up through the DatabaseHolder is a bit of a layering
+    // violation, but at this point we're not going to add more MMAPv1 specific interfaces.
+    StringData dbName = systemCollectionNamespace.db();
+    invariant(txn->lockState()->isDbLockedForMode(dbName, MODE_X));
+    Database* db = dbHolder().get(txn, dbName);
+    Collection* systemCollection = db->getCollection(systemCollectionNamespace);
+    systemCollection->getCursorManager()->invalidateDocument(txn, record, INVALIDATION_DELETION);
+}
+
 void MMAPV1DatabaseCatalogEntry::appendExtraStats(OperationContext* opCtx,
                                                   BSONObjBuilder* output,
                                                   double scale) const {
@@ -822,13 +823,8 @@ void MMAPV1DatabaseCatalogEntry::_removeNamespaceFromNamespaceCollection(Operati
 
     // Invalidate old namespace record
     RecordId oldSpecLocation = entry->second->catalogEntry->getNamespacesRecordId();
-    const NamespaceString nsn(name(), "system.namespaces");
-    StringData dbName(name());
-    invariant(txn->lockState()->isDbLockedForMode(dbName, MODE_X));
-    Database* db = dbHolder().get(txn, dbName);
-    Collection* systemNamespaces = db->getCollection(nsn);
-    systemNamespaces->getCursorManager()->invalidateDocument(
-        txn, oldSpecLocation, INVALIDATION_DELETION);
+    invalidateSystemCollectionRecord(
+        txn, NamespaceString(name(), "system.namespaces"), oldSpecLocation);
 
     rs->deleteRecord(txn, oldSpecLocation);
 }
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h
index 250d8084b00..1f0ee7dfa38 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_database_catalog_entry.h
@@ -123,11 +123,16 @@ public:
     CollectionOptions getCollectionOptions(OperationContext* txn, RecordId nsRid) const;
 
     /**
-     * Creates a CollectionCatalogEntry in the for an index rather than a collection. MMAPv1
-     * puts both indexes and collections into CCEs. A namespace named 'name' must not exist.
+     * Creates a CollectionCatalogEntry in the form of an index rather than a collection.
+     * MMAPv1 puts both indexes and collections into CCEs. A namespace named 'name' must not
+     * exist.
      */
     void createNamespaceForIndex(OperationContext* txn, const StringData& name);
 
+    static void invalidateSystemCollectionRecord(OperationContext* txn,
+                                                 NamespaceString systemCollectionNamespace,
+                                                 RecordId record);
+
 private:
     class EntryInsertion;
     class EntryRemoval;