From 71c05d54c1e0f05e4e99ceda9ee7deb3c6a6b247 Mon Sep 17 00:00:00 2001 From: Geert Bosch Date: Thu, 4 May 2017 18:36:07 -0400 Subject: SERVER-29088 Cache uuid in Collection class to fix perf regression --- src/mongo/db/catalog/capped_utils.cpp | 4 ++-- src/mongo/db/catalog/coll_mod.cpp | 2 +- src/mongo/db/catalog/collection.cpp | 3 ++- src/mongo/db/catalog/collection.h | 10 ++++++---- src/mongo/db/catalog/collection_impl.cpp | 11 +++++++---- src/mongo/db/catalog/collection_impl.h | 6 ++++-- src/mongo/db/catalog/collection_mock.h | 2 +- src/mongo/db/catalog/database_impl.cpp | 6 +++--- src/mongo/db/catalog/drop_indexes.cpp | 6 +++--- src/mongo/db/catalog/rename_collection.cpp | 6 +++--- src/mongo/db/cloner.cpp | 2 +- src/mongo/db/commands/create_indexes.cpp | 2 +- src/mongo/db/commands/feature_compatibility_version.cpp | 2 +- src/mongo/db/commands/mr.cpp | 2 +- src/mongo/db/exec/update.cpp | 2 +- src/mongo/db/repair_database.cpp | 3 ++- src/mongo/db/s/migration_destination_manager.cpp | 7 ++----- 17 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/mongo/db/catalog/capped_utils.cpp b/src/mongo/db/catalog/capped_utils.cpp index f893b2f8cf5..cd2fa1700e9 100644 --- a/src/mongo/db/catalog/capped_utils.cpp +++ b/src/mongo/db/catalog/capped_utils.cpp @@ -103,7 +103,7 @@ Status emptyCapped(OperationContext* opCtx, const NamespaceString& collectionNam } getGlobalServiceContext()->getOpObserver()->onEmptyCapped( - opCtx, collection->ns(), collection->uuid(opCtx)); + opCtx, collection->ns(), collection->uuid()); wuow.commit(); @@ -282,7 +282,7 @@ Status convertToCapped(OperationContext* opCtx, } } - OptionalCollectionUUID uuid = db->getCollection(opCtx, longTmpName)->uuid(opCtx); + OptionalCollectionUUID uuid = db->getCollection(opCtx, longTmpName)->uuid(); { WriteUnitOfWork wunit(opCtx); diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index c1feeff855e..3d8ca6cd721 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -388,7 +388,7 @@ Status collMod(OperationContext* opCtx, // Only observe non-view collMods, as view operations are observed as operations on the // system.views collection. getGlobalServiceContext()->getOpObserver()->onCollMod( - opCtx, nss, coll->uuid(opCtx), oplogEntryBuilder.obj(), oldCollOptions, ttlInfo); + opCtx, nss, coll->uuid(), oplogEntryBuilder.obj(), oldCollOptions, ttlInfo); wunit.commit(); diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp index c9b6ad39f19..07054e8c03a 100644 --- a/src/mongo/db/catalog/collection.cpp +++ b/src/mongo/db/catalog/collection.cpp @@ -84,10 +84,11 @@ void Collection::registerFactory(decltype(factory) newFactory) { auto Collection::makeImpl(Collection* _this, OperationContext* const opCtx, const StringData fullNS, + OptionalCollectionUUID uuid, CollectionCatalogEntry* const details, RecordStore* const recordStore, DatabaseCatalogEntry* const dbce) -> std::unique_ptr { - return factory(_this, opCtx, fullNS, details, recordStore, dbce); + return factory(_this, opCtx, fullNS, uuid, details, recordStore, dbce); } void Collection::TUHook::hook() noexcept {} diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h index 25fb8930290..caf3a32288e 100644 --- a/src/mongo/db/catalog/collection.h +++ b/src/mongo/db/catalog/collection.h @@ -199,7 +199,7 @@ public: virtual const CollectionInfoCache* infoCache() const = 0; virtual const NamespaceString& ns() const = 0; - virtual OptionalCollectionUUID uuid(OperationContext* opCtx) const = 0; + virtual OptionalCollectionUUID uuid() const = 0; virtual const IndexCatalog* getIndexCatalog() const = 0; virtual IndexCatalog* getIndexCatalog() = 0; @@ -322,6 +322,7 @@ private: static std::unique_ptr makeImpl(Collection* _this, OperationContext* opCtx, StringData fullNS, + OptionalCollectionUUID uuid, CollectionCatalogEntry* details, RecordStore* recordStore, DatabaseCatalogEntry* dbce); @@ -333,10 +334,11 @@ public: explicit inline Collection(OperationContext* const opCtx, const StringData fullNS, + OptionalCollectionUUID uuid, CollectionCatalogEntry* const details, // does not own RecordStore* const recordStore, // does not own DatabaseCatalogEntry* const dbce) // does not own - : _pimpl(makeImpl(this, opCtx, fullNS, details, recordStore, dbce)) { + : _pimpl(makeImpl(this, opCtx, fullNS, uuid, details, recordStore, dbce)) { this->_impl().init(opCtx); } @@ -369,8 +371,8 @@ public: return this->_impl().ns(); } - inline OptionalCollectionUUID uuid(OperationContext* opCtx) const { - return this->_impl().uuid(opCtx); + inline OptionalCollectionUUID uuid() const { + return this->_impl().uuid(); } inline const IndexCatalog* getIndexCatalog() const { diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp index 0d46e479171..4840f77828e 100644 --- a/src/mongo/db/catalog/collection_impl.cpp +++ b/src/mongo/db/catalog/collection_impl.cpp @@ -80,11 +80,12 @@ MONGO_INITIALIZER(InitializeCollectionFactory)(InitializerContext* const) { [](Collection* const _this, OperationContext* const opCtx, const StringData fullNS, + OptionalCollectionUUID uuid, CollectionCatalogEntry* const details, RecordStore* const recordStore, DatabaseCatalogEntry* const dbce) -> std::unique_ptr { return stdx::make_unique( - _this, opCtx, fullNS, details, recordStore, dbce); + _this, opCtx, fullNS, uuid, details, recordStore, dbce); }); return Status::OK(); } @@ -237,10 +238,12 @@ bool CappedInsertNotifier::isDead() { CollectionImpl::CollectionImpl(Collection* _this_init, OperationContext* opCtx, StringData fullNS, + OptionalCollectionUUID uuid, CollectionCatalogEntry* details, RecordStore* recordStore, DatabaseCatalogEntry* dbce) : _ns(fullNS), + _uuid(uuid), _details(details), _recordStore(recordStore), _dbce(dbce), @@ -444,7 +447,7 @@ Status CollectionImpl::insertDocuments(OperationContext* opCtx, invariant(sid == opCtx->recoveryUnit()->getSnapshotId()); getGlobalServiceContext()->getOpObserver()->onInserts( - opCtx, ns(), uuid(opCtx), begin, end, fromMigrate); + opCtx, ns(), uuid(), begin, end, fromMigrate); opCtx->recoveryUnit()->onCommit([this]() { notifyCappedWaitersIfNeeded(); }); @@ -507,7 +510,7 @@ Status CollectionImpl::insertDocument(OperationContext* opCtx, docs.push_back(doc); getGlobalServiceContext()->getOpObserver()->onInserts( - opCtx, ns(), uuid(opCtx), docs.begin(), docs.end(), false); + opCtx, ns(), uuid(), docs.begin(), docs.end(), false); opCtx->recoveryUnit()->onCommit([this]() { notifyCappedWaitersIfNeeded(); }); @@ -622,7 +625,7 @@ void CollectionImpl::deleteDocument( _recordStore->deleteRecord(opCtx, loc); getGlobalServiceContext()->getOpObserver()->onDelete( - opCtx, ns(), uuid(opCtx), std::move(deleteState), fromMigrate); + opCtx, ns(), uuid(), std::move(deleteState), fromMigrate); } Counter64 moveCounter; diff --git a/src/mongo/db/catalog/collection_impl.h b/src/mongo/db/catalog/collection_impl.h index f5d2cb1d50e..043b98ef424 100644 --- a/src/mongo/db/catalog/collection_impl.h +++ b/src/mongo/db/catalog/collection_impl.h @@ -42,6 +42,7 @@ public: explicit CollectionImpl(Collection* _this, OperationContext* opCtx, StringData fullNS, + OptionalCollectionUUID uuid, CollectionCatalogEntry* details, // does not own RecordStore* recordStore, // does not own DatabaseCatalogEntry* dbce); // does not own @@ -74,8 +75,8 @@ public: return _ns; } - OptionalCollectionUUID uuid(OperationContext* opCtx) const { - return getCatalogEntry()->getCollectionOptions(opCtx).uuid; + OptionalCollectionUUID uuid() const { + return _uuid; } const IndexCatalog* getIndexCatalog() const final { @@ -386,6 +387,7 @@ private: int _magic; const NamespaceString _ns; + OptionalCollectionUUID _uuid; CollectionCatalogEntry* const _details; RecordStore* const _recordStore; DatabaseCatalogEntry* const _dbce; diff --git a/src/mongo/db/catalog/collection_mock.h b/src/mongo/db/catalog/collection_mock.h index 7b46367a766..8fe55e28a1d 100644 --- a/src/mongo/db/catalog/collection_mock.h +++ b/src/mongo/db/catalog/collection_mock.h @@ -269,7 +269,7 @@ public: std::abort(); } - OptionalCollectionUUID uuid(OperationContext* opCtx) const { + OptionalCollectionUUID uuid() const { std::abort(); } }; diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index ab8ba32ca7f..518508663e5 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -211,13 +211,13 @@ Collection* DatabaseImpl::_getOrCreateCollectionInstance(OperationContext* opCtx } unique_ptr cce(_dbEntry->getCollectionCatalogEntry(nss.ns())); - invariant(cce.get()); + auto uuid = cce->getCollectionOptions(opCtx).uuid; unique_ptr rs(_dbEntry->getRecordStore(nss.ns())); invariant(rs.get()); // if cce exists, so should this // Not registering AddCollectionChange since this is for collections that already exist. - Collection* c = new Collection(opCtx, nss.ns(), cce.release(), rs.release(), _dbEntry); + Collection* c = new Collection(opCtx, nss.ns(), uuid, cce.release(), rs.release(), _dbEntry); return c; } @@ -439,7 +439,7 @@ Status DatabaseImpl::dropCollectionEvenIfSystem(OperationContext* opCtx, // We want to destroy the Collection object before telling the StorageEngine to destroy the // RecordStore. - auto uuid = collection->uuid(opCtx); + auto uuid = collection->uuid(); _clearCollectionCache(opCtx, fullns.toString(), "collection dropped"); s = _dbEntry->dropCollection(opCtx, fullns.toString()); diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp index 657d35f8f88..3ef36e6dbd7 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -70,7 +70,7 @@ Status wrappedRun(OperationContext* opCtx, // We log one op for every dropped index so that we can roll them back if necessary. for (auto const& idx : droppedIndexes) { opCtx->getServiceContext()->getOpObserver()->onDropIndex( - opCtx, collection->ns(), collection->uuid(opCtx), idx.first, idx.second); + opCtx, collection->ns(), collection->uuid(), idx.first, idx.second); } anObjBuilder->append("msg", "non-_id indexes dropped for collection"); @@ -94,7 +94,7 @@ Status wrappedRun(OperationContext* opCtx, } opCtx->getServiceContext()->getOpObserver()->onDropIndex( - opCtx, collection->ns(), collection->uuid(opCtx), desc->indexName(), desc->infoObj()); + opCtx, collection->ns(), collection->uuid(), desc->indexName(), desc->infoObj()); return Status::OK(); } @@ -128,7 +128,7 @@ Status wrappedRun(OperationContext* opCtx, } opCtx->getServiceContext()->getOpObserver()->onDropIndex( - opCtx, collection->ns(), collection->uuid(opCtx), desc->indexName(), desc->infoObj()); + opCtx, collection->ns(), collection->uuid(), desc->indexName(), desc->infoObj()); return Status::OK(); } diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index 37e35387249..3a7475dc276 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -144,7 +144,7 @@ Status renameCollection(OperationContext* opCtx, str::stream() << "a view already exists with that name: " << target.ns()); } - auto sourceUUID = sourceColl->uuid(opCtx); + auto sourceUUID = sourceColl->uuid(); // If we are renaming in the same database, just rename the namespace and we're done. if (sourceDB == targetDB) { MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN { @@ -153,7 +153,7 @@ Status renameCollection(OperationContext* opCtx, if (targetColl) { // No logOp necessary because the entire renameCollection command is one logOp. repl::UnreplicatedWritesBlock uwb(opCtx); - dropTargetUUID = targetColl->uuid(opCtx); + dropTargetUUID = targetColl->uuid(); Status s = targetDB->dropCollection(opCtx, target.ns()); if (!s.isOK()) { return s; @@ -280,7 +280,7 @@ Status renameCollection(OperationContext* opCtx, repl::UnreplicatedWritesBlock uwb(opCtx); Status status = Status::OK(); if (targetColl) { - dropTargetUUID = targetColl->uuid(opCtx); + dropTargetUUID = targetColl->uuid(); status = targetDB->dropCollection(opCtx, target.ns()); } if (status.isOK()) diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp index 66acfee5228..0d294656632 100644 --- a/src/mongo/db/cloner.cpp +++ b/src/mongo/db/cloner.cpp @@ -411,7 +411,7 @@ void Cloner::copyIndexes(OperationContext* opCtx, if (opCtx->writesAreReplicated()) { for (auto&& infoObj : indexInfoObjs) { getGlobalServiceContext()->getOpObserver()->onCreateIndex( - opCtx, collection->ns(), collection->uuid(opCtx), infoObj, false); + opCtx, collection->ns(), collection->uuid(), infoObj, false); } } wunit.commit(); diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index 9b91e2a3dfc..db71d3bebf5 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -390,7 +390,7 @@ public: for (auto&& infoObj : indexInfoObjs) { getGlobalServiceContext()->getOpObserver()->onCreateIndex( - opCtx, ns, collection->uuid(opCtx), infoObj, false); + opCtx, ns, collection->uuid(), infoObj, false); } wunit.commit(); diff --git a/src/mongo/db/commands/feature_compatibility_version.cpp b/src/mongo/db/commands/feature_compatibility_version.cpp index c227c498e12..4fd5a6573e6 100644 --- a/src/mongo/db/commands/feature_compatibility_version.cpp +++ b/src/mongo/db/commands/feature_compatibility_version.cpp @@ -233,7 +233,7 @@ void FeatureCompatibilityVersion::set(OperationContext* opCtx, StringData versio MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN { WriteUnitOfWork wuow(opCtx); - auto uuid = autoDB.getDb()->getCollection(opCtx, nss)->uuid(opCtx); + auto uuid = autoDB.getDb()->getCollection(opCtx, nss)->uuid(); getGlobalServiceContext()->getOpObserver()->onCreateIndex( opCtx, nss, uuid, k32IncompatibleIndexSpec, false); wuow.commit(); diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 8993e12db51..6e5213052ca 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -515,7 +515,7 @@ void State::prepTempCollection() { uassertStatusOK(status); } // Log the createIndex operation. - auto uuid = tempColl->uuid(_opCtx); + auto uuid = tempColl->uuid(); getGlobalServiceContext()->getOpObserver()->onCreateIndex( _opCtx, _config.tempNamespace, uuid, *it, false); } diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp index 32011d751a1..9bcc5460290 100644 --- a/src/mongo/db/exec/update.cpp +++ b/src/mongo/db/exec/update.cpp @@ -696,7 +696,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted& oldObj, Reco BSONObj idQuery = driver->makeOplogEntryQuery(newObj, request->isMulti()); OplogUpdateEntryArgs args; args.nss = _collection->ns(); - args.uuid = _collection->uuid(getOpCtx()); + args.uuid = _collection->uuid(); args.update = logObj; args.criteria = idQuery; args.fromMigrate = request->isFromMigration(); diff --git a/src/mongo/db/repair_database.cpp b/src/mongo/db/repair_database.cpp index 615604ba8dc..70405da7928 100644 --- a/src/mongo/db/repair_database.cpp +++ b/src/mongo/db/repair_database.cpp @@ -143,7 +143,8 @@ Status rebuildIndexesOnCollection(OperationContext* opCtx, // open a bad index and fail. // TODO see if MultiIndexBlock can be made to work without a Collection. const StringData ns = cce->ns().ns(); - collection.reset(new Collection(opCtx, ns, cce, dbce->getRecordStore(ns), dbce)); + const auto uuid = cce->getCollectionOptions(opCtx).uuid; + collection.reset(new Collection(opCtx, ns, uuid, cce, dbce->getRecordStore(ns), dbce)); indexer.reset(new MultiIndexBlock(opCtx, collection.get())); Status status = indexer->init(indexSpecs).getStatus(); diff --git a/src/mongo/db/s/migration_destination_manager.cpp b/src/mongo/db/s/migration_destination_manager.cpp index f8787766da0..beb61bfd3db 100644 --- a/src/mongo/db/s/migration_destination_manager.cpp +++ b/src/mongo/db/s/migration_destination_manager.cpp @@ -587,11 +587,8 @@ void MigrationDestinationManager::_migrateDriver(OperationContext* opCtx, for (auto&& infoObj : indexInfoObjs.getValue()) { // make sure to create index on secondaries as well - getGlobalServiceContext()->getOpObserver()->onCreateIndex(opCtx, - collection->ns(), - collection->uuid(opCtx), - infoObj, - true /* fromMigrate */); + getGlobalServiceContext()->getOpObserver()->onCreateIndex( + opCtx, collection->ns(), collection->uuid(), infoObj, true /* fromMigrate */); } wunit.commit(); -- cgit v1.2.1