summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHenrik Edin <henrik.edin@mongodb.com>2022-05-04 10:02:01 -0400
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2022-05-19 15:09:07 +0000
commit686303404d6faa0dde943deea549005baf46efb4 (patch)
treece524e6ada25a9bfb71fbb32197edf101f35c615
parent177ac5a512463db8f6f2d8ba1eefe052a52a8f37 (diff)
downloadmongo-686303404d6faa0dde943deea549005baf46efb4.tar.gz
SERVER-66176 Reduce work under CollectionCatalog lock
* Perform hashing before taking lock. * Add new API for restoring after yield where we can lookup by UUID and verify namespace in the same critical section. Avoid two calls and copying the namespace under the lock. (cherry picked from commit 7e0f7d50da0915428a3bfa10a656cd999d5fa10e)
-rw-r--r--src/mongo/db/catalog/collection_catalog.cpp35
-rw-r--r--src/mongo/db/catalog/collection_catalog.h12
-rw-r--r--src/mongo/db/exec/requires_collection_stage.cpp15
3 files changed, 48 insertions, 14 deletions
diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index ba77219fe29..3df5856faab 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -234,24 +234,46 @@ void CollectionCatalog::onOpenCatalog(OperationContext* opCtx) {
}
Collection* CollectionCatalog::lookupCollectionByUUID(CollectionUUID uuid) const {
+ size_t hash = _catalog.hash_function()(uuid);
stdx::lock_guard<Latch> lock(_catalogLock);
- return _lookupCollectionByUUID(lock, uuid);
+ return _lookupCollectionByUUID(lock, uuid, hash);
}
-Collection* CollectionCatalog::_lookupCollectionByUUID(WithLock, CollectionUUID uuid) const {
- auto foundIt = _catalog.find(uuid);
+std::pair<Collection*, boost::optional<NamespaceString>>
+CollectionCatalog::lookupCollectionByUUIDAndVerifyNamespace(CollectionUUID uuid,
+ const NamespaceString& nss) const {
+ size_t hash = _catalog.hash_function()(uuid);
+ stdx::lock_guard<Latch> lock(_catalogLock);
+ auto coll = _lookupCollectionByUUID(lock, uuid, hash);
+ if (coll) {
+ bool renamed = coll->ns() != nss;
+ if (renamed) {
+ return {nullptr, coll->ns()};
+ } else {
+ return {coll, boost::none};
+ }
+ }
+ return {nullptr, boost::none};
+}
+
+Collection* CollectionCatalog::_lookupCollectionByUUID(WithLock,
+ CollectionUUID uuid,
+ size_t hash) const {
+ auto foundIt = _catalog.find(uuid, hash);
return foundIt == _catalog.end() ? nullptr : foundIt->second.get();
}
Collection* CollectionCatalog::lookupCollectionByNamespace(const NamespaceString& nss) const {
+ size_t hash = _collections.hash_function()(nss);
stdx::lock_guard<Latch> lock(_catalogLock);
- auto it = _collections.find(nss);
+ auto it = _collections.find(nss, hash);
return it == _collections.end() ? nullptr : it->second;
}
boost::optional<NamespaceString> CollectionCatalog::lookupNSSByUUID(CollectionUUID uuid) const {
+ size_t hash = _catalog.hash_function()(uuid);
stdx::lock_guard<Latch> lock(_catalogLock);
- auto foundIt = _catalog.find(uuid);
+ auto foundIt = _catalog.find(uuid, hash);
if (foundIt != _catalog.end()) {
NamespaceString ns = foundIt->second->ns();
invariant(!ns.isEmpty());
@@ -290,8 +312,9 @@ bool CollectionCatalog::checkIfCollectionSatisfiable(CollectionUUID uuid,
CollectionInfoFn predicate) const {
invariant(predicate);
+ size_t hash = _catalog.hash_function()(uuid);
stdx::lock_guard<Latch> lock(_catalogLock);
- auto collection = _lookupCollectionByUUID(lock, uuid);
+ auto collection = _lookupCollectionByUUID(lock, uuid, hash);
if (!collection) {
return false;
diff --git a/src/mongo/db/catalog/collection_catalog.h b/src/mongo/db/catalog/collection_catalog.h
index 8ad0128e744..ce145b8d689 100644
--- a/src/mongo/db/catalog/collection_catalog.h
+++ b/src/mongo/db/catalog/collection_catalog.h
@@ -143,6 +143,16 @@ public:
Collection* lookupCollectionByUUID(CollectionUUID uuid) const;
/**
+ * Like lookupCollectionByUUID but also verifies the namespace under the catalog lock. Used when
+ * not certain we're holding the right Collection lock.
+ *
+ * The optional NamespaceString is set when the Collection was found but did not pass the
+ * namespace check, none otherwise.
+ */
+ std::pair<Collection*, boost::optional<NamespaceString>>
+ lookupCollectionByUUIDAndVerifyNamespace(CollectionUUID uuid, const NamespaceString& nss) const;
+
+ /**
* This function gets the Collection pointer that corresponds to the NamespaceString.
* The required locks must be obtained prior to calling this function, or else the found
* Collection pointer may no longer be valid when the call returns.
@@ -237,7 +247,7 @@ public:
private:
friend class CollectionCatalog::iterator;
- Collection* _lookupCollectionByUUID(WithLock, CollectionUUID uuid) const;
+ Collection* _lookupCollectionByUUID(WithLock, CollectionUUID uuid, size_t hash) const;
const std::vector<CollectionUUID>& _getOrdering_inlock(const StringData& db,
const stdx::lock_guard<Latch>&);
diff --git a/src/mongo/db/exec/requires_collection_stage.cpp b/src/mongo/db/exec/requires_collection_stage.cpp
index 060722dbe14..6c57f37574b 100644
--- a/src/mongo/db/exec/requires_collection_stage.cpp
+++ b/src/mongo/db/exec/requires_collection_stage.cpp
@@ -52,23 +52,24 @@ void RequiresCollectionStageBase<CollectionT>::doRestoreState() {
dassert(getOpCtx()->lockState()->isCollectionLockedForMode(_nss, MODE_IS));
const CollectionCatalog& catalog = CollectionCatalog::get(getOpCtx());
- auto newNss = catalog.lookupNSSByUUID(_collectionUUID);
+ Collection* coll = nullptr;
+ boost::optional<NamespaceString> newNss;
+ std::tie(coll, newNss) =
+ catalog.lookupCollectionByUUIDAndVerifyNamespace(_collectionUUID, _nss);
uassert(ErrorCodes::QueryPlanKilled,
str::stream() << "collection dropped. UUID " << _collectionUUID,
- newNss);
+ coll || newNss);
// TODO SERVER-31695: Allow queries to survive collection rename, rather than throwing here when
// a rename has happened during yield.
uassert(ErrorCodes::QueryPlanKilled,
str::stream() << "collection renamed from '" << _nss << "' to '" << *newNss
<< "'. UUID " << _collectionUUID,
- *newNss == _nss);
+ coll);
// At this point we know that the collection name has not changed, and therefore we have
- // restored locks on the correct name. It is now safe to restore the Collection pointer. The
- // collection must exist, since we already successfully looked up the namespace string by UUID
- // under the correct lock manager locks.
- _collection = catalog.lookupCollectionByUUID(_collectionUUID);
+ // restored locks on the correct name.
+ _collection = coll;
invariant(_collection);
uassert(ErrorCodes::QueryPlanKilled,