diff options
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; |