diff options
4 files changed, 73 insertions, 46 deletions
diff --git a/jstests/core/list_namespaces_invalidation.js b/jstests/core/list_namespaces_invalidation.js index 81f6c7d69f1..6f8033b5fe4 100644 --- a/jstests/core/list_namespaces_invalidation.js +++ b/jstests/core/list_namespaces_invalidation.js @@ -1,10 +1,13 @@ -// 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) { + let DROP = 1; + let RENAME = 2; + let MOVE = 3; + function testNamespaceInvalidation(namespaceAction, batchSize) { dbInvalid.dropDatabase(); // Create enough collections to necessitate multiple cursor batches. @@ -16,30 +19,55 @@ // otherwise. let cmd = dbInvalid.system.indexes.count() ? {find: 'system.namespaces'} : {listCollections: dbInvalidName}; - Object.extend(cmd, {batchSize: 2}); + Object.extend(cmd, {batchSize: batchSize}); let res = dbInvalid.runCommand(cmd); assert.commandWorked(res, 'could not run ' + tojson(cmd)); printjson(res); - // Ensure the cursor has data, drop or rename the collections, and exhaust the cursor. + // Ensure the cursor has data, invalidate the namespace, and exhaust the cursor. let cursor = new DBCommandCursor(dbInvalid.getMongo(), res); let errMsg = 'expected more data from command ' + tojson(cmd) + ', with result ' + tojson(res); 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()); + } else if (namespaceAction == MOVE) { + let modCmd = { + collMod: 'coll1', + validator: { + $or: [ + {phone: {$type: "string"}}, + {email: {$regex: /@mongodb\.com$/}}, + {status: {$in: ["Unknown", "Incomplete"]}}, + {address: {$type: "string"}}, + {ssn: {$type: "string"}}, + {favoriteBook: {$type: "string"}}, + {favoriteColor: {$type: "string"}}, + {favoriteBeverage: {$type: "string"}}, + {favoriteDay: {$type: "string"}}, + {favoriteFood: {$type: "string"}}, + {favoriteSport: {$type: "string"}}, + {favoriteMovie: {$type: "string"}}, + {favoriteShow: {$type: "string"}} + ] + } + }; + assert.commandWorked(dbInvalid.runCommand(modCmd)); } 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, rename, or move a + // namespace record. + for (let j = 2; j < 7; j++) { + testNamespaceInvalidation(DROP, j); + testNamespaceInvalidation(RENAME, j); + testNamespaceInvalidation(MOVE, j); + } }()); 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 7a54f771fa6..a2421962853 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 @@ -259,15 +259,10 @@ Status NamespaceDetailsCollectionCatalogEntry::removeIndex(OperationContext* txn d->idx(getTotalIndexCount(txn)) = IndexDetails(); } - // Soneone may be querying the system.indexes namespace directly, so we need to invalidate - // its cursors. 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. - // We can find the name of the database through the first entries of the CollectionMap. - StringData dbName(nsToDatabaseSubstring(_db->_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, infoLocation, INVALIDATION_DELETION); + // 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); @@ -394,6 +389,10 @@ void NamespaceDetailsCollectionCatalogEntry::_updateSystemNamespaces(OperationCo txn, newEntry.objdata(), newEntry.objsize(), false); fassert(40074, newLocation.getStatus().isOK()); + // Invalidate old namespace record + MMAPV1DatabaseCatalogEntry::invalidateSystemCollectionRecord( + txn, NamespaceString(_db->name(), "system.namespaces"), _namespacesRecordId); + _namespacesRecordStore->deleteRecord(txn, _namespacesRecordId); setNamespacesRecordId(txn, newLocation.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 51e07a797a3..3c8a4de47d5 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 @@ -310,12 +310,8 @@ 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"), record->id); systemIndexRecordStore->deleteRecord(txn, record->id); } @@ -383,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); @@ -403,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 { @@ -860,13 +862,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 8d6c16ee8ea..5934ded8a11 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 @@ -132,6 +132,9 @@ public: * exist. */ void createNamespaceForIndex(OperationContext* txn, StringData name); + static void invalidateSystemCollectionRecord(OperationContext* txn, + NamespaceString systemCollectionNamespace, + RecordId record); private: class EntryInsertion; |