diff options
23 files changed, 223 insertions, 282 deletions
diff --git a/src/mongo/db/catalog/catalog_helper.cpp b/src/mongo/db/catalog/catalog_helper.cpp index c8307dcdf7d..b5f1e078ec5 100644 --- a/src/mongo/db/catalog/catalog_helper.cpp +++ b/src/mongo/db/catalog/catalog_helper.cpp @@ -46,17 +46,14 @@ void assertMatchingDbVersion(OperationContext* opCtx, const StringData& dbName) } { - const auto dss = DatabaseShardingState::getSharedForLockFreeReads(opCtx, dbName); - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss.get()); - - const auto critSecSignal = dss->getCriticalSectionSignal( + auto scopedDss = DatabaseShardingState::acquire(opCtx, dbName, DSSAcquisitionMode::kShared); + const auto critSecSignal = scopedDss->getCriticalSectionSignal( opCtx->lockState()->isWriteLocked() ? ShardingMigrationCriticalSection::kWrite - : ShardingMigrationCriticalSection::kRead, - dssLock); + : ShardingMigrationCriticalSection::kRead); uassert( StaleDbRoutingVersion(dbName.toString(), *receivedVersion, boost::none, critSecSignal), str::stream() << "The critical section for the database " << dbName - << " is acquired with reason: " << dss->getCriticalSectionReason(dssLock), + << " is acquired with reason: " << scopedDss->getCriticalSectionReason(), !critSecSignal); } diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index c3c4a2284ca..ef743884654 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -27,13 +27,9 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include "mongo/db/catalog/coll_mod.h" #include <boost/optional.hpp> -#include <memory> #include "mongo/db/catalog/clustered_collection_util.h" #include "mongo/db/catalog/coll_mod_index.h" @@ -67,30 +63,25 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kCommand - namespace mongo { - namespace { MONGO_FAIL_POINT_DEFINE(hangAfterDatabaseLock); MONGO_FAIL_POINT_DEFINE(hangAfterCollModIndexUniqueFullIndexScan); MONGO_FAIL_POINT_DEFINE(hangAfterCollModIndexUniqueReleaseIXLock); -void assertMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { - auto dss = DatabaseShardingState::get(opCtx, nss.db().toString()); - if (!dss) { - return; - } - - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss); +void assertNoMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { try { - const auto collDesc = - CollectionShardingState::get(opCtx, nss)->getCollectionDescription(opCtx); - if (!collDesc.isSharded()) { - auto mpsm = dss->getMovePrimarySourceManager(dssLock); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, nss.dbName(), DSSAcquisitionMode::kShared); - if (mpsm) { - LOGV2(4945200, "assertMovePrimaryInProgress", "namespace"_attr = nss.toString()); + auto css = CollectionShardingState::get(opCtx, nss); + auto collDesc = css->getCollectionDescription(opCtx); + collDesc.throwIfReshardingInProgress(nss); + + if (!collDesc.isSharded()) { + if (scopedDss->isMovePrimaryInProgress()) { + LOGV2(4945200, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); uasserted(ErrorCodes::MovePrimaryInProgress, "movePrimary is in progress for namespace " + nss.toString()); @@ -746,14 +737,10 @@ Status _collModInternal(OperationContext* opCtx, // This can kill all cursors so don't allow running it while a background operation is in // progress. if (coll) { - assertMovePrimaryInProgress(opCtx, nss); + assertNoMovePrimaryInProgress(opCtx, nss); IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection(coll->uuid()); - CollectionShardingState::get(opCtx, nss) - ->getCollectionDescription(opCtx) - .throwIfReshardingInProgress(nss); } - // If db/collection/view does not exist, short circuit and return. if (!db || (!coll && !view)) { if (nss.isTimeseriesBucketsCollection()) { diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index 28c3fa2e8cb..bbb91165272 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -112,18 +112,11 @@ Status validateDBNameForWindows(StringData dbname) { return Status::OK(); } -void assertMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { - invariant(opCtx->lockState()->isDbLockedForMode(nss.dbName(), MODE_IS)); - auto dss = DatabaseShardingState::get(opCtx, nss.dbName().toStringWithTenantId()); - if (!dss) { - return; - } - - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss); - auto mpsm = dss->getMovePrimarySourceManager(dssLock); - - if (mpsm) { - LOGV2(4909100, "assertMovePrimaryInProgress", "namespace"_attr = nss.toString()); +void assertNoMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + if (scopedDss->isMovePrimaryInProgress()) { + LOGV2(4909100, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); uasserted(ErrorCodes::MovePrimaryInProgress, "movePrimary is in progress for namespace " + nss.toString()); @@ -666,7 +659,7 @@ Status DatabaseImpl::renameCollection(OperationContext* opCtx, return Status(ErrorCodes::NamespaceNotFound, "collection not found to rename"); } - assertMovePrimaryInProgress(opCtx, fromNss); + assertNoMovePrimaryInProgress(opCtx, fromNss); LOGV2(20319, "renameCollection: renaming collection {collToRename_uuid} from {fromNss} to {toNss}", @@ -827,7 +820,7 @@ Collection* DatabaseImpl::createCollection(OperationContext* opCtx, }); _checkCanCreateCollection(opCtx, nss, optionsWithUUID); - assertMovePrimaryInProgress(opCtx, nss); + assertNoMovePrimaryInProgress(opCtx, nss); audit::logCreateCollection(opCtx->getClient(), nss); LOGV2(20320, diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp index ecbbf35893c..8ee4b3cc43f 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -27,9 +27,6 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include "mongo/db/catalog/drop_indexes.h" #include <boost/algorithm/string/join.hpp> @@ -55,7 +52,6 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kCommand - namespace mongo { namespace { @@ -366,21 +362,21 @@ void dropReadyIndexes(OperationContext* opCtx, } } -void assertMovePrimaryInProgress(OperationContext* opCtx, const NamespaceString& ns) { - auto dss = DatabaseShardingState::get(opCtx, ns.db()); - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss); - +void assertNoMovePrimaryInProgress(OperationContext* opCtx, const NamespaceString& nss) { try { - const auto collDesc = - CollectionShardingState::get(opCtx, ns)->getCollectionDescription(opCtx); - if (!collDesc.isSharded()) { - auto mpsm = dss->getMovePrimarySourceManager(dssLock); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + + auto css = CollectionShardingState::get(opCtx, nss); + auto collDesc = css->getCollectionDescription(opCtx); + collDesc.throwIfReshardingInProgress(nss); - if (mpsm) { - LOGV2(4976500, "assertMovePrimaryInProgress", "namespace"_attr = ns.toString()); + if (!collDesc.isSharded()) { + if (scopedDss->isMovePrimaryInProgress()) { + LOGV2(4976500, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); uasserted(ErrorCodes::MovePrimaryInProgress, - "movePrimary is in progress for namespace " + ns.toString()); + "movePrimary is in progress for namespace " + nss.toString()); } } } catch (const DBException& ex) { @@ -492,10 +488,7 @@ DropIndexesReply dropIndexes(OperationContext* opCtx, } if (!abortAgain) { - assertMovePrimaryInProgress(opCtx, collNs); - CollectionShardingState::get(opCtx, collNs) - ->getCollectionDescription(opCtx) - .throwIfReshardingInProgress(collNs); + assertNoMovePrimaryInProgress(opCtx, collNs); break; } } diff --git a/src/mongo/db/catalog_raii.cpp b/src/mongo/db/catalog_raii.cpp index 30e9224024a..a02ae9838cc 100644 --- a/src/mongo/db/catalog_raii.cpp +++ b/src/mongo/db/catalog_raii.cpp @@ -27,9 +27,6 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include "mongo/db/catalog_raii.h" #include "mongo/db/catalog/catalog_helper.h" @@ -44,13 +41,11 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kStorage - -MONGO_FAIL_POINT_DEFINE(hangBeforeAutoGetCollectionLockFreeShardedStateAccess); - namespace mongo { namespace { MONGO_FAIL_POINT_DEFINE(setAutoGetCollectionWait); +MONGO_FAIL_POINT_DEFINE(hangBeforeAutoGetCollectionLockFreeShardedStateAccess); /** * Performs some sanity checks on the collection and database. diff --git a/src/mongo/db/commands/SConscript b/src/mongo/db/commands/SConscript index 6461f4dffc6..c26e7dfc7c0 100644 --- a/src/mongo/db/commands/SConscript +++ b/src/mongo/db/commands/SConscript @@ -309,7 +309,7 @@ env.Library( "analyze_cmd.cpp", "count_cmd.cpp", "create_command.cpp", - "create_indexes.cpp", + "create_indexes_cmd.cpp", "current_op.cpp", "dbcommands.cpp", "distinct.cpp", diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes_cmd.cpp index fc4c24786f8..233d60bba58 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes_cmd.cpp @@ -27,9 +27,6 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include <string> #include <vector> @@ -77,10 +74,9 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kIndex - namespace mongo { - namespace { + // This failpoint simulates a WriteConflictException during createIndexes where the collection is // implicitly created. MONGO_FAIL_POINT_DEFINE(createIndexesWriteConflict); @@ -313,25 +309,20 @@ bool indexesAlreadyExist(OperationContext* opCtx, return true; } -/** - * Checks database sharding state. Throws exception on error. - */ -void checkDatabaseShardingState(OperationContext* opCtx, const NamespaceString& ns) { - auto dss = DatabaseShardingState::get(opCtx, ns.db()); - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss); - - Lock::CollectionLock collLock(opCtx, ns, MODE_IS); +void assertNoMovePrimaryInProgress(OperationContext* opCtx, const NamespaceString& nss) { try { - const auto collDesc = - CollectionShardingState::get(opCtx, ns)->getCollectionDescription(opCtx); - if (!collDesc.isSharded()) { - auto mpsm = dss->getMovePrimarySourceManager(dssLock); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, nss.dbName(), DSSAcquisitionMode::kShared); - if (mpsm) { - LOGV2(4909200, "assertMovePrimaryInProgress", "namespace"_attr = ns.toString()); + Lock::CollectionLock collLock(opCtx, nss, MODE_IX); + + auto collDesc = CollectionShardingState::get(opCtx, nss)->getCollectionDescription(opCtx); + if (!collDesc.isSharded()) { + if (scopedDss->isMovePrimaryInProgress()) { + LOGV2(4909200, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); uasserted(ErrorCodes::MovePrimaryInProgress, - "movePrimary is in progress for namespace " + ns.toString()); + "movePrimary is in progress for namespace " + nss.toString()); } } } catch (const DBException& ex) { @@ -492,8 +483,8 @@ CreateIndexesReply runCreateIndexesWithCoordinator(OperationContext* opCtx, CreateIndexesReply reply; { AutoGetDb autoDb(opCtx, ns.db(), MODE_IX); + assertNoMovePrimaryInProgress(opCtx, ns); - checkDatabaseShardingState(opCtx, ns); if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, ns)) { uasserted(ErrorCodes::NotWritablePrimary, str::stream() << "Not primary while creating indexes in " << ns.ns()); diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 3ce99611f15..c410f390fe6 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -27,9 +27,10 @@ * it in the license file. */ - #include "mongo/db/index_builds_coordinator.h" +#include <boost/filesystem/operations.hpp> +#include <boost/iterator/transform_iterator.hpp> #include <fmt/format.h> #include "mongo/db/catalog/clustered_collection_util.h" @@ -67,12 +68,8 @@ #include "mongo/util/scoped_counter.h" #include "mongo/util/str.h" -#include <boost/filesystem/operations.hpp> -#include <boost/iterator/transform_iterator.hpp> - #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kStorage - namespace mongo { MONGO_FAIL_POINT_DEFINE(hangAfterIndexBuildFirstDrain); diff --git a/src/mongo/db/s/database_sharding_state.cpp b/src/mongo/db/s/database_sharding_state.cpp index ab9545ad96c..da908b6ea38 100644 --- a/src/mongo/db/s/database_sharding_state.cpp +++ b/src/mongo/db/s/database_sharding_state.cpp @@ -27,22 +27,16 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include "mongo/db/s/database_sharding_state.h" #include "mongo/db/operation_context.h" #include "mongo/db/s/operation_sharding_state.h" -#include "mongo/db/s/sharding_state.h" #include "mongo/logv2/log.h" -#include "mongo/s/database_version.h" -#include "mongo/s/stale_exception.h" +#include "mongo/stdx/unordered_map.h" #include "mongo/util/fail_point.h" #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kSharding - namespace mongo { namespace { @@ -55,7 +49,7 @@ public: DatabaseShardingStateMap() {} - std::shared_ptr<DatabaseShardingState> getOrCreate(const StringData dbName) { + DatabaseShardingState* getOrCreate(const DatabaseName& dbName) { stdx::lock_guard<Latch> lg(_mutex); auto it = _databases.find(dbName); @@ -66,13 +60,13 @@ public: it = std::move(inserted.first); } - return it->second; + return it->second.get(); } private: - using DatabasesMap = StringMap<std::shared_ptr<DatabaseShardingState>>; - Mutex _mutex = MONGO_MAKE_LATCH("DatabaseShardingStateMap::_mutex"); + + using DatabasesMap = stdx::unordered_map<DatabaseName, std::unique_ptr<DatabaseShardingState>>; DatabasesMap _databases; }; @@ -81,54 +75,55 @@ const ServiceContext::Decoration<DatabaseShardingStateMap> DatabaseShardingState } // namespace -DatabaseShardingState::DatabaseShardingState(const StringData dbName) - : _dbName(dbName.toString()) {} +DatabaseShardingState::ScopedDatabaseShardingState::ScopedDatabaseShardingState( + OperationContext* opCtx, const DatabaseName& dbName, LockMode mode) + : _lock(nullptr, opCtx->lockState(), ResourceId(RESOURCE_MUTEX, dbName), mode), + _dss(DatabaseShardingStateMap::get(opCtx->getServiceContext()).getOrCreate(dbName)) {} -DatabaseShardingState* DatabaseShardingState::get(OperationContext* opCtx, - const StringData dbName) { - // db lock must be held to have a reference to the database sharding state - // TODO SERVER-63706 Use dbName directly - dassert(opCtx->lockState()->isDbLockedForMode(DatabaseName(boost::none, dbName), MODE_IS)); +DatabaseShardingState::ScopedDatabaseShardingState::ScopedDatabaseShardingState( + ScopedDatabaseShardingState&& other) + : _lock(std::move(other._lock)), _dss(other._dss) { + other._dss = nullptr; +} - auto& databasesMap = DatabaseShardingStateMap::get(opCtx->getServiceContext()); - return databasesMap.getOrCreate(dbName).get(); +DatabaseShardingState::ScopedDatabaseShardingState::~ScopedDatabaseShardingState() = default; + +DatabaseShardingState::DatabaseShardingState(const DatabaseName& dbName) : _dbName(dbName) {} + +DatabaseShardingState::ScopedDatabaseShardingState DatabaseShardingState::assertDbLockedAndAcquire( + OperationContext* opCtx, const DatabaseName& dbName, DSSAcquisitionMode mode) { + dassert(opCtx->lockState()->isDbLockedForMode(dbName, MODE_IS)); + + return acquire(opCtx, dbName, mode); } -std::shared_ptr<DatabaseShardingState> DatabaseShardingState::getSharedForLockFreeReads( - OperationContext* opCtx, const StringData dbName) { - auto& databasesMap = DatabaseShardingStateMap::get(opCtx->getServiceContext()); - return databasesMap.getOrCreate(dbName); +DatabaseShardingState::ScopedDatabaseShardingState DatabaseShardingState::acquire( + OperationContext* opCtx, const DatabaseName& dbName, DSSAcquisitionMode mode) { + return ScopedDatabaseShardingState( + opCtx, dbName, mode == DSSAcquisitionMode::kShared ? MODE_IS : MODE_X); } void DatabaseShardingState::enterCriticalSectionCatchUpPhase(OperationContext* opCtx, - DSSLock& dssLock, const BSONObj& reason) { - invariant(opCtx->lockState()->isDbLockedForMode(DatabaseName(boost::none, _dbName), MODE_X)); + invariant(opCtx->lockState()->isDbLockedForMode(_dbName, MODE_X)); _critSec.enterCriticalSectionCatchUpPhase(reason); - cancelDbMetadataRefresh(dssLock); + cancelDbMetadataRefresh(); } void DatabaseShardingState::enterCriticalSectionCommitPhase(OperationContext* opCtx, - DSSLock&, const BSONObj& reason) { - invariant(opCtx->lockState()->isDbLockedForMode(DatabaseName(boost::none, _dbName), MODE_X)); + invariant(opCtx->lockState()->isDbLockedForMode(_dbName, MODE_X)); _critSec.enterCriticalSectionCommitPhase(reason); } void DatabaseShardingState::exitCriticalSection(OperationContext* opCtx, const BSONObj& reason) { - const auto dssLock = DSSLock::lockExclusive(opCtx, this); _critSec.exitCriticalSection(reason); } -MovePrimarySourceManager* DatabaseShardingState::getMovePrimarySourceManager(DSSLock&) { - return _sourceMgr; -} - void DatabaseShardingState::setMovePrimarySourceManager(OperationContext* opCtx, - MovePrimarySourceManager* sourceMgr, - DSSLock&) { - invariant(opCtx->lockState()->isDbLockedForMode(DatabaseName(boost::none, _dbName), MODE_X)); + MovePrimarySourceManager* sourceMgr) { + invariant(opCtx->lockState()->isDbLockedForMode(_dbName, MODE_X)); invariant(sourceMgr); invariant(!_sourceMgr); @@ -136,29 +131,26 @@ void DatabaseShardingState::setMovePrimarySourceManager(OperationContext* opCtx, } void DatabaseShardingState::clearMovePrimarySourceManager(OperationContext* opCtx) { - invariant(opCtx->lockState()->isDbLockedForMode(DatabaseName(boost::none, _dbName), MODE_IX)); - const auto dssLock = DSSLock::lockExclusive(opCtx, this); + invariant(opCtx->lockState()->isDbLockedForMode(_dbName, MODE_IX)); _sourceMgr = nullptr; } void DatabaseShardingState::setDbMetadataRefreshFuture(SharedSemiFuture<void> future, - CancellationSource cancellationSource, - const DSSLock&) { + CancellationSource cancellationSource) { invariant(!_dbMetadataRefresh); _dbMetadataRefresh.emplace(std::move(future), std::move(cancellationSource)); } -boost::optional<SharedSemiFuture<void>> DatabaseShardingState::getDbMetadataRefreshFuture( - const DSSLock&) const { +boost::optional<SharedSemiFuture<void>> DatabaseShardingState::getDbMetadataRefreshFuture() const { return _dbMetadataRefresh ? boost::optional<SharedSemiFuture<void>>(_dbMetadataRefresh->future) : boost::none; } -void DatabaseShardingState::resetDbMetadataRefreshFuture(const DSSLock&) { +void DatabaseShardingState::resetDbMetadataRefreshFuture() { _dbMetadataRefresh = boost::none; } -void DatabaseShardingState::cancelDbMetadataRefresh(const DSSLock&) { +void DatabaseShardingState::cancelDbMetadataRefresh() { if (_dbMetadataRefresh) { _dbMetadataRefresh->cancellationSource.cancel(); } diff --git a/src/mongo/db/s/database_sharding_state.h b/src/mongo/db/s/database_sharding_state.h index 56d896a2720..5018a873c9c 100644 --- a/src/mongo/db/s/database_sharding_state.h +++ b/src/mongo/db/s/database_sharding_state.h @@ -30,14 +30,15 @@ #pragma once #include "mongo/bson/bsonobj.h" +#include "mongo/db/concurrency/d_concurrency.h" #include "mongo/db/s/sharding_migration_critical_section.h" -#include "mongo/db/s/sharding_state_lock.h" #include "mongo/s/catalog/type_database_gen.h" namespace mongo { class MovePrimarySourceManager; -class OperationContext; + +enum class DSSAcquisitionMode { kShared, kExclusive }; /** * Synchronizes access to this shard server's cached database version for Database. @@ -47,36 +48,47 @@ class DatabaseShardingState { DatabaseShardingState& operator=(const DatabaseShardingState&) = delete; public: - /** - * A ShardingStateLock is used on DatabaseShardingState operations in order to ensure - * synchronization across operations. - */ - using DSSLock = ShardingStateLock<DatabaseShardingState>; - - DatabaseShardingState(StringData dbName); + DatabaseShardingState(const DatabaseName& dbName); ~DatabaseShardingState() = default; /** - * Obtains the sharding state for the specified database. If it does not exist, it will be - * created and will remain in memory until the database is dropped. - * - * Must be called with some lock held on the database being looked up and the returned - * pointer must not be stored. + * Obtains the sharding state for the specified database along with a resource lock protecting + * it from modifications, which will be held until the object goes out of scope. */ - static DatabaseShardingState* get(OperationContext* opCtx, StringData dbName); + class ScopedDatabaseShardingState { + public: + ScopedDatabaseShardingState(ScopedDatabaseShardingState&&); - /** - * Obtain a pointer to the DatabaseShardingState that remains safe to access without holding - * a database lock. Should be called instead of the regular get() if no database lock is held. - * The returned DatabaseShardingState instance should not be modified! - */ - static std::shared_ptr<DatabaseShardingState> getSharedForLockFreeReads(OperationContext* opCtx, - StringData dbName); + ~ScopedDatabaseShardingState(); + + DatabaseShardingState* operator->() const { + return _dss; + } + DatabaseShardingState& operator*() const { + return *_dss; + } + + private: + friend class DatabaseShardingState; + + ScopedDatabaseShardingState(OperationContext* opCtx, + const DatabaseName& dbName, + LockMode mode); + + Lock::ResourceLock _lock; + DatabaseShardingState* _dss; + }; + static ScopedDatabaseShardingState assertDbLockedAndAcquire(OperationContext* opCtx, + const DatabaseName& dbName, + DSSAcquisitionMode mode); + static ScopedDatabaseShardingState acquire(OperationContext* opCtx, + const DatabaseName& dbName, + DSSAcquisitionMode mode); /** * Returns the name of the database related to the current sharding state. */ - std::string getDbName() const { + const DatabaseName& getDbName() const { return _dbName; } @@ -84,31 +96,31 @@ public: * Methods to control the databases's critical section. Must be called with the database X lock * held. */ - void enterCriticalSectionCatchUpPhase(OperationContext* opCtx, DSSLock&, const BSONObj& reason); - void enterCriticalSectionCommitPhase(OperationContext* opCtx, DSSLock&, const BSONObj& reason); + void enterCriticalSectionCatchUpPhase(OperationContext* opCtx, const BSONObj& reason); + void enterCriticalSectionCommitPhase(OperationContext* opCtx, const BSONObj& reason); void exitCriticalSection(OperationContext* opCtx, const BSONObj& reason); - auto getCriticalSectionSignal(ShardingMigrationCriticalSection::Operation op, DSSLock&) const { + auto getCriticalSectionSignal(ShardingMigrationCriticalSection::Operation op) const { return _critSec.getSignal(op); } - auto getCriticalSectionReason(DSSLock&) const { + auto getCriticalSectionReason() const { return _critSec.getReason() ? _critSec.getReason()->toString() : "Unknown"; } /** * Returns the active movePrimary source manager, if one is available. */ - MovePrimarySourceManager* getMovePrimarySourceManager(DSSLock&); + bool isMovePrimaryInProgress() const { + return _sourceMgr; + } /** * Attaches a movePrimary source manager to this database's sharding state. Must be called with * the database lock in X mode. May not be called if there is a movePrimary source manager * already installed. Must be followed by a call to clearMovePrimarySourceManager. */ - void setMovePrimarySourceManager(OperationContext* opCtx, - MovePrimarySourceManager* sourceMgr, - DSSLock&); + void setMovePrimarySourceManager(OperationContext* opCtx, MovePrimarySourceManager* sourceMgr); /** * Removes a movePrimary source manager from this database's sharding state. Must be called with @@ -121,28 +133,25 @@ public: * Sets the database metadata refresh future for other threads to wait on it. */ void setDbMetadataRefreshFuture(SharedSemiFuture<void> future, - CancellationSource cancellationSource, - const DSSLock&); + CancellationSource cancellationSource); /** * If there is an ongoing database metadata refresh, returns the future to wait on it, otherwise * `boost::none`. */ - boost::optional<SharedSemiFuture<void>> getDbMetadataRefreshFuture(const DSSLock&) const; + boost::optional<SharedSemiFuture<void>> getDbMetadataRefreshFuture() const; /** * Resets the database metadata refresh future to `boost::none`. */ - void resetDbMetadataRefreshFuture(const DSSLock&); + void resetDbMetadataRefreshFuture(); /** * Cancel any ongoing database metadata refresh. */ - void cancelDbMetadataRefresh(const DSSLock&); + void cancelDbMetadataRefresh(); private: - friend DSSLock; - struct DbMetadataRefresh { DbMetadataRefresh(SharedSemiFuture<void> future, CancellationSource cancellationSource) : future(std::move(future)), cancellationSource(std::move(cancellationSource)){}; @@ -158,7 +167,7 @@ private: // within. Lock::ResourceMutex _stateChangeMutex{"DatabaseShardingState"}; - const std::string _dbName; + const DatabaseName _dbName; // Modifying the state below requires holding the DBLock in X mode; holding the DBLock in any // mode is acceptable for reading it. (Note: accessing this class at all requires holding the diff --git a/src/mongo/db/s/drop_database_coordinator.cpp b/src/mongo/db/s/drop_database_coordinator.cpp index 64c365d784a..349d9ae4314 100644 --- a/src/mongo/db/s/drop_database_coordinator.cpp +++ b/src/mongo/db/s/drop_database_coordinator.cpp @@ -89,10 +89,10 @@ public: // directly DatabaseName databaseName(boost::none, _dbName); Lock::DBLock dbLock(_opCtx, databaseName, MODE_X); - auto dss = DatabaseShardingState::get(_opCtx, _dbName); - auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(_opCtx, dss); - dss->enterCriticalSectionCatchUpPhase(_opCtx, dssLock, _reason); - dss->enterCriticalSectionCommitPhase(_opCtx, dssLock, _reason); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + _opCtx, databaseName, DSSAcquisitionMode::kExclusive); + scopedDss->enterCriticalSectionCatchUpPhase(_opCtx, _reason); + scopedDss->enterCriticalSectionCommitPhase(_opCtx, _reason); } ~ScopedDatabaseCriticalSection() { @@ -101,8 +101,9 @@ public: // directly DatabaseName databaseName(boost::none, _dbName); Lock::DBLock dbLock(_opCtx, databaseName, MODE_X); - auto dss = DatabaseShardingState::get(_opCtx, _dbName); - dss->exitCriticalSection(_opCtx, _reason); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + _opCtx, databaseName, DSSAcquisitionMode::kExclusive); + scopedDss->exitCriticalSection(_opCtx, _reason); } private: diff --git a/src/mongo/db/s/flush_database_cache_updates_command.cpp b/src/mongo/db/s/flush_database_cache_updates_command.cpp index ff4837fdf16..be96e307fe2 100644 --- a/src/mongo/db/s/flush_database_cache_updates_command.cpp +++ b/src/mongo/db/s/flush_database_cache_updates_command.cpp @@ -173,10 +173,10 @@ public: // inclusive of the commit (and new writes to the committed chunk) that hasn't yet // propagated back to this shard. This ensures the read your own writes causal // consistency guarantee. - const auto dss = DatabaseShardingState::get(opCtx, _dbName()); - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss); + const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, ns().dbName(), DSSAcquisitionMode::kShared); criticalSectionSignal = - dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead, dssLock); + scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead); } if (criticalSectionSignal) diff --git a/src/mongo/db/s/move_primary_source_manager.cpp b/src/mongo/db/s/move_primary_source_manager.cpp index d870dc4e644..688c3fddd8f 100644 --- a/src/mongo/db/s/move_primary_source_manager.cpp +++ b/src/mongo/db/s/move_primary_source_manager.cpp @@ -34,6 +34,7 @@ #include "mongo/db/catalog_raii.h" #include "mongo/db/commands.h" #include "mongo/db/dbdirectclient.h" +#include "mongo/db/s/database_sharding_state.h" #include "mongo/db/s/shard_metadata_util.h" #include "mongo/db/s/sharding_logging.h" #include "mongo/db/s/sharding_state_recovery.h" @@ -104,10 +105,9 @@ Status MovePrimarySourceManager::clone(OperationContext* opCtx) { AutoGetDb autoDb(opCtx, getNss().dbName(), MODE_X); invariant(autoDb.ensureDbExists(opCtx), getNss().toString()); - auto dss = DatabaseShardingState::get(opCtx, getNss().toString()); - auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); - - dss->setMovePrimarySourceManager(opCtx, this, dssLock); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + scopedDss->setMovePrimarySourceManager(opCtx, this); } _state = kCloning; @@ -173,11 +173,11 @@ Status MovePrimarySourceManager::enterCriticalSection(OperationContext* opCtx) { << " was dropped during the movePrimary operation."); } - auto dss = DatabaseShardingState::get(opCtx, getNss().toString()); - auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); // IMPORTANT: After this line, the critical section is in place and needs to be signaled - dss->enterCriticalSectionCatchUpPhase(opCtx, dssLock, _critSecReason); + scopedDss->enterCriticalSectionCatchUpPhase(opCtx, _critSecReason); } _state = kCriticalSection; @@ -224,12 +224,12 @@ Status MovePrimarySourceManager::commitOnConfig(OperationContext* opCtx) { << " was dropped during the movePrimary operation."); } - auto dss = DatabaseShardingState::get(opCtx, getNss().toString()); - auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); // Read operations must begin to wait on the critical section just before we send the // commit operation to the config server - dss->enterCriticalSectionCommitPhase(opCtx, dssLock, _critSecReason); + scopedDss->enterCriticalSectionCommitPhase(opCtx, _critSecReason); expectedDbVersion = DatabaseHolder::get(opCtx)->getDbVersion(opCtx, _dbname); } @@ -505,12 +505,13 @@ void MovePrimarySourceManager::_cleanup(OperationContext* opCtx) { UninterruptibleLockGuard noInterrupt(opCtx->lockState()); AutoGetDb autoDb(opCtx, getNss().dbName(), MODE_IX); - auto dss = DatabaseShardingState::get(opCtx, getNss().db()); - dss->clearMovePrimarySourceManager(opCtx); - DatabaseHolder::get(opCtx)->clearDbInfo(opCtx, - DatabaseName(boost::none, getNss().toString())); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + scopedDss->clearMovePrimarySourceManager(opCtx); + DatabaseHolder::get(opCtx)->clearDbInfo(opCtx, getNss().dbName()); + // Leave the critical section if we're still registered. - dss->exitCriticalSection(opCtx, _critSecReason); + scopedDss->exitCriticalSection(opCtx, _critSecReason); } if (_state == kCriticalSection || _state == kCloneCompleted) { diff --git a/src/mongo/db/s/op_observer_sharding_impl.cpp b/src/mongo/db/s/op_observer_sharding_impl.cpp index 5f83126597b..ce1c2e94d09 100644 --- a/src/mongo/db/s/op_observer_sharding_impl.cpp +++ b/src/mongo/db/s/op_observer_sharding_impl.cpp @@ -79,7 +79,7 @@ bool isMigratingWithCSRLock(CollectionShardingRuntime* csr, return cloner && cloner->isDocumentInMigratingChunk(docToDelete); } -void assertMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { +void assertNoMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { if (!nss.isNormalCollection() && nss.coll() != "system.views" && !nss.isTimeseriesBucketsCollection()) { return; @@ -88,16 +88,11 @@ void assertMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& // TODO SERVER-58222: evaluate whether this is safe or whether acquiring the lock can block. AllowLockAcquisitionOnTimestampedUnitOfWork allowLockAcquisition(opCtx->lockState()); Lock::DBLock dblock(opCtx, nss.dbName(), MODE_IS); - auto dss = DatabaseShardingState::get(opCtx, nss.db().toString()); - if (!dss) { - return; - } - - auto dssLock = DatabaseShardingState::DSSLock::lockShared(opCtx, dss); - auto mpsm = dss->getMovePrimarySourceManager(dssLock); - if (mpsm) { - LOGV2(4908600, "assertMovePrimaryInProgress", "namespace"_attr = nss.toString()); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + if (scopedDss->isMovePrimaryInProgress()) { + LOGV2(4908600, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); uasserted(ErrorCodes::MovePrimaryInProgress, "movePrimary is in progress for namespace " + nss.toString()); @@ -139,7 +134,7 @@ void OpObserverShardingImpl::shardObserveInsertOp(OperationContext* opCtx, auto metadata = csr->getCurrentMetadataIfKnown(); if (!metadata || !metadata->isSharded()) { - assertMovePrimaryInProgress(opCtx, nss); + assertNoMovePrimaryInProgress(opCtx, nss); return; } @@ -176,7 +171,7 @@ void OpObserverShardingImpl::shardObserveUpdateOp(OperationContext* opCtx, auto metadata = csr->getCurrentMetadataIfKnown(); if (!metadata || !metadata->isSharded()) { - assertMovePrimaryInProgress(opCtx, nss); + assertNoMovePrimaryInProgress(opCtx, nss); return; } @@ -212,7 +207,7 @@ void OpObserverShardingImpl::shardObserveDeleteOp(OperationContext* opCtx, auto metadata = csr->getCurrentMetadataIfKnown(); if (!metadata || !metadata->isSharded()) { - assertMovePrimaryInProgress(opCtx, nss); + assertNoMovePrimaryInProgress(opCtx, nss); return; } diff --git a/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp b/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp index 8b7fb280c33..22dae7b285d 100644 --- a/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp +++ b/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp @@ -27,14 +27,12 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include "mongo/db/s/resharding/resharding_donor_recipient_common.h" #include <fmt/format.h> #include "mongo/db/persistent_task_store.h" +#include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/shard_filtering_metadata_refresh.h" #include "mongo/db/s/sharding_state.h" #include "mongo/db/storage/duplicate_key_error_info.h" @@ -45,7 +43,6 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kResharding - namespace mongo { namespace resharding { diff --git a/src/mongo/db/s/resharding/resharding_recipient_service.cpp b/src/mongo/db/s/resharding/resharding_recipient_service.cpp index fe0d54112ea..012a70221ea 100644 --- a/src/mongo/db/s/resharding/resharding_recipient_service.cpp +++ b/src/mongo/db/s/resharding/resharding_recipient_service.cpp @@ -43,7 +43,6 @@ #include "mongo/db/repl/oplog_applier.h" #include "mongo/db/repl/read_concern_args.h" #include "mongo/db/repl/wait_for_majority_service.h" -#include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/migration_destination_manager.h" #include "mongo/db/s/resharding/resharding_change_event_o2_field_gen.h" #include "mongo/db/s/resharding/resharding_data_copy_util.h" diff --git a/src/mongo/db/s/resharding/resharding_recipient_service_external_state.cpp b/src/mongo/db/s/resharding/resharding_recipient_service_external_state.cpp index 222a2c6f86a..9c718a2290b 100644 --- a/src/mongo/db/s/resharding/resharding_recipient_service_external_state.cpp +++ b/src/mongo/db/s/resharding/resharding_recipient_service_external_state.cpp @@ -27,9 +27,9 @@ * it in the license file. */ - #include "mongo/db/s/resharding/resharding_recipient_service_external_state.h" +#include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/resharding/resharding_donor_recipient_common.h" #include "mongo/db/s/sharding_state.h" #include "mongo/db/write_concern_options.h" @@ -42,7 +42,6 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kResharding - namespace mongo { namespace { diff --git a/src/mongo/db/s/resharding/resharding_recipient_service_external_state_test.cpp b/src/mongo/db/s/resharding/resharding_recipient_service_external_state_test.cpp index b004e7178ef..7ce77e0329e 100644 --- a/src/mongo/db/s/resharding/resharding_recipient_service_external_state_test.cpp +++ b/src/mongo/db/s/resharding/resharding_recipient_service_external_state_test.cpp @@ -35,6 +35,7 @@ #include "mongo/db/repl/oplog.h" #include "mongo/db/repl/replication_coordinator_mock.h" #include "mongo/db/repl/storage_interface_impl.h" +#include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/resharding/resharding_oplog_applier_progress_gen.h" #include "mongo/db/s/resharding/resharding_recipient_service_external_state.h" #include "mongo/db/s/resharding/resharding_util.h" diff --git a/src/mongo/db/s/resharding/resharding_util.h b/src/mongo/db/s/resharding/resharding_util.h index 6ed3319fecd..7c021858971 100644 --- a/src/mongo/db/s/resharding/resharding_util.h +++ b/src/mongo/db/s/resharding/resharding_util.h @@ -26,23 +26,19 @@ * exception statement from all source files in the program, then also delete * it in the license file. */ + #pragma once #include <vector> #include "mongo/bson/bsonobj.h" -#include "mongo/bson/timestamp.h" -#include "mongo/db/catalog/database.h" #include "mongo/db/catalog_raii.h" #include "mongo/db/keypattern.h" #include "mongo/db/operation_context.h" #include "mongo/db/pipeline/pipeline.h" #include "mongo/db/repl/primary_only_service.h" -#include "mongo/db/s/collection_sharding_runtime.h" #include "mongo/db/s/resharding/coordinator_document_gen.h" #include "mongo/db/s/resharding/donor_oplog_id_gen.h" -#include "mongo/db/s/sharding_state_lock.h" -#include "mongo/db/shard_id.h" #include "mongo/executor/task_executor.h" #include "mongo/s/catalog/type_tags.h" #include "mongo/s/chunk_manager.h" diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp index ecf39cb7296..e156770ebb3 100644 --- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp +++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp @@ -64,36 +64,36 @@ MONGO_FAIL_POINT_DEFINE(hangInRecoverRefreshThread); * Returns 'true' if there were concurrent operations that had to be joined (in which case all locks * will be dropped). If there were none, returns false and the locks continue to be held. */ -bool joinDbVersionOperation(OperationContext* opCtx, - DatabaseShardingState* dss, - boost::optional<Lock::DBLock>* dbLock, - boost::optional<DatabaseShardingState::DSSLock>* dssLock) { +bool joinDbVersionOperation( + OperationContext* opCtx, + boost::optional<Lock::DBLock>* dbLock, + boost::optional<DatabaseShardingState::ScopedDatabaseShardingState>* scopedDss) { invariant(dbLock->has_value()); - invariant(dssLock->has_value()); + invariant(scopedDss->has_value()); if (auto critSect = - dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead, **dssLock)) { + (**scopedDss)->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)) { LOGV2_DEBUG(6697201, 2, "Waiting for exit from the critical section", - "db"_attr = dss->getDbName(), - "reason"_attr = dss->getCriticalSectionReason(**dssLock)); + "db"_attr = (**scopedDss)->getDbName(), + "reason"_attr = (**scopedDss)->getCriticalSectionReason()); + scopedDss->reset(); dbLock->reset(); - dssLock->reset(); uassertStatusOK(OperationShardingState::waitForCriticalSectionToComplete(opCtx, *critSect)); return true; } - if (auto refreshVersionFuture = dss->getDbMetadataRefreshFuture(**dssLock)) { + if (auto refreshVersionFuture = (**scopedDss)->getDbMetadataRefreshFuture()) { LOGV2_DEBUG(6697202, 2, "Waiting for completion of another database metadata refresh", - "db"_attr = dss->getDbName()); + "db"_attr = (**scopedDss)->getDbName()); + scopedDss->reset(); dbLock->reset(); - dssLock->reset(); try { refreshVersionFuture->get(opCtx); @@ -122,11 +122,10 @@ Status refreshDbMetadata(OperationContext* opCtx, ScopeGuard resetRefreshFutureOnError([&] { UninterruptibleLockGuard noInterrupt(opCtx->lockState()); - Lock::DBLock dbLock(opCtx, dbName, MODE_IS); - auto* dss = DatabaseShardingState::get(opCtx, dbName.db()); - const auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); - - dss->resetDbMetadataRefreshFuture(dssLock); + Lock::DBLock dbLock(opCtx, dbName, MODE_IX); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, dbName, DSSAcquisitionMode::kExclusive); + scopedDss->resetDbMetadataRefreshFuture(); }); // Force a refresh of the cached database metadata from the config server. @@ -134,9 +133,8 @@ Status refreshDbMetadata(OperationContext* opCtx, Grid::get(opCtx)->catalogCache()->getDatabaseWithRefresh(opCtx, dbName.db()); Lock::DBLock dbLock(opCtx, dbName, MODE_X); - auto* dss = DatabaseShardingState::get(opCtx, dbName.db()); - const auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); - + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, dbName, DSSAcquisitionMode::kExclusive); if (!cancellationToken.isCanceled()) { if (swDbMetadata.isOK()) { // Set the refreshed database metadata in the local catalog. @@ -149,7 +147,7 @@ Status refreshDbMetadata(OperationContext* opCtx, } // Reset the future reference to allow any other thread to refresh the database metadata. - dss->resetDbMetadataRefreshFuture(dssLock); + scopedDss->resetDbMetadataRefreshFuture(); resetRefreshFutureOnError.dismiss(); return swDbMetadata.getStatus(); @@ -224,14 +222,15 @@ void onDbVersionMismatch(OperationContext* opCtx, boost::optional<SharedSemiFuture<void>> dbMetadataRefreshFuture; { - auto dbLock = boost::make_optional(Lock::DBLock(opCtx, dbName, MODE_IS)); - auto* dss = DatabaseShardingState::get(opCtx, dbName); + boost::optional<Lock::DBLock> dbLock; + dbLock.emplace(opCtx, dbName, MODE_IS); if (receivedDbVersion) { - auto dssLock = - boost::make_optional(DatabaseShardingState::DSSLock::lockShared(opCtx, dss)); + boost::optional<DatabaseShardingState::ScopedDatabaseShardingState> scopedDss( + DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, dbName, DSSAcquisitionMode::kShared)); - if (joinDbVersionOperation(opCtx, dss, &dbLock, &dssLock)) { + if (joinDbVersionOperation(opCtx, &dbLock, &scopedDss)) { // Waited for another thread to exit from the critical section or to complete an // ongoing refresh, so reacquire the locks. continue; @@ -242,9 +241,8 @@ void onDbVersionMismatch(OperationContext* opCtx, // is in progress or can start (would require to exclusive lock the DSS). // Therefore, the database version can be accessed safely. - const auto wantedDbVersion = - DatabaseHolder::get(opCtx)->getDbVersion(opCtx, dbName); - if (receivedDbVersion <= wantedDbVersion) { + const auto wantedVersion = DatabaseHolder::get(opCtx)->getDbVersion(opCtx, dbName); + if (receivedDbVersion <= wantedVersion) { // No need to refresh the database metadata as the wanted version is newer // than the one received. return; @@ -255,10 +253,11 @@ void onDbVersionMismatch(OperationContext* opCtx, return; } - auto dssLock = - boost::make_optional(DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss)); + boost::optional<DatabaseShardingState::ScopedDatabaseShardingState> scopedDss( + DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, dbName, DSSAcquisitionMode::kExclusive)); - if (joinDbVersionOperation(opCtx, dss, &dbLock, &dssLock)) { + if (joinDbVersionOperation(opCtx, &dbLock, &scopedDss)) { // Waited for another thread to exit from the critical section or to complete an // ongoing refresh, so reacquire the locks. continue; @@ -271,11 +270,11 @@ void onDbVersionMismatch(OperationContext* opCtx, CancellationSource cancellationSource; CancellationToken cancellationToken = cancellationSource.token(); - dss->setDbMetadataRefreshFuture( - recoverRefreshDbVersion(opCtx, dbName, cancellationToken), - std::move(cancellationSource), - *dssLock); - dbMetadataRefreshFuture = dss->getDbMetadataRefreshFuture(*dssLock); + (*scopedDss) + ->setDbMetadataRefreshFuture( + recoverRefreshDbVersion(opCtx, dbName, cancellationToken), + std::move(cancellationSource)); + dbMetadataRefreshFuture = (*scopedDss)->getDbMetadataRefreshFuture(); } // No other metadata refresh for this database can run in parallel. If another thread enters diff --git a/src/mongo/db/s/shard_server_op_observer.cpp b/src/mongo/db/s/shard_server_op_observer.cpp index d86597bd86b..32ad318a1de 100644 --- a/src/mongo/db/s/shard_server_op_observer.cpp +++ b/src/mongo/db/s/shard_server_op_observer.cpp @@ -415,12 +415,14 @@ void ShardServerOpObserver::onUpdate(OperationContext* opCtx, const OplogUpdateE // block. AllowLockAcquisitionOnTimestampedUnitOfWork allowLockAcquisition(opCtx->lockState()); - AutoGetDb autoDb(opCtx, DatabaseName(boost::none, db), MODE_X); - DatabaseHolder::get(opCtx)->clearDbInfo(opCtx, DatabaseName(boost::none, db)); + DatabaseName dbName(boost::none, db); - auto* dss = DatabaseShardingState::get(opCtx, db); - const auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); - dss->cancelDbMetadataRefresh(dssLock); + AutoGetDb autoDb(opCtx, dbName, MODE_X); + DatabaseHolder::get(opCtx)->clearDbInfo(opCtx, dbName); + + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, dbName, DSSAcquisitionMode::kExclusive); + scopedDss->cancelDbMetadataRefresh(); } } @@ -532,12 +534,14 @@ void ShardServerOpObserver::onDelete(OperationContext* opCtx, // TODO SERVER-58223: evaluate whether this is safe or whether acquiring the lock can block. AllowLockAcquisitionOnTimestampedUnitOfWork allowLockAcquisition(opCtx->lockState()); - AutoGetDb autoDb(opCtx, DatabaseName(boost::none, deletedDatabase), MODE_X); - DatabaseHolder::get(opCtx)->clearDbInfo(opCtx, DatabaseName(boost::none, deletedDatabase)); + DatabaseName dbName(boost::none, deletedDatabase); + + AutoGetDb autoDb(opCtx, dbName, MODE_X); + DatabaseHolder::get(opCtx)->clearDbInfo(opCtx, dbName); - auto* dss = DatabaseShardingState::get(opCtx, deletedDatabase); - const auto dssLock = DatabaseShardingState::DSSLock::lockExclusive(opCtx, dss); - dss->cancelDbMetadataRefresh(dssLock); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( + opCtx, dbName, DSSAcquisitionMode::kExclusive); + scopedDss->cancelDbMetadataRefresh(); } if (nss == NamespaceString::kServerConfigurationNamespace) { diff --git a/src/mongo/db/s/sharding_state_lock.h b/src/mongo/db/s/sharding_state_lock.h index 88f1350edae..07fa1d8a430 100644 --- a/src/mongo/db/s/sharding_state_lock.h +++ b/src/mongo/db/s/sharding_state_lock.h @@ -41,7 +41,6 @@ namespace mongo { */ template <class ShardingState> class ShardingStateLock { - public: /** * Locks the sharding state object with the sharding state object's ResourceMutex in MODE_IS. diff --git a/src/mongo/db/s/split_vector_test.cpp b/src/mongo/db/s/split_vector_test.cpp index d1e7ae54829..11fb9eba29a 100644 --- a/src/mongo/db/s/split_vector_test.cpp +++ b/src/mongo/db/s/split_vector_test.cpp @@ -27,9 +27,6 @@ * it in the license file. */ - -#include "mongo/platform/basic.h" - #include "mongo/db/catalog/create_collection.h" #include "mongo/db/db_raii.h" #include "mongo/db/dbdirectclient.h" @@ -40,7 +37,6 @@ #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kTest - namespace mongo { namespace { |