From f139c94b7ae8961f5b18df0e95e9708b3710929b Mon Sep 17 00:00:00 2001 From: Antonio Fuschetto Date: Thu, 9 Feb 2023 11:59:18 +0000 Subject: SERVER-72346 Enforce the required lock mode in the DatabaseShardingState API --- src/mongo/db/catalog/coll_mod.cpp | 4 +- src/mongo/db/catalog/database_impl.cpp | 4 +- src/mongo/db/catalog/drop_indexes.cpp | 4 +- src/mongo/db/commands/create_indexes_cmd.cpp | 4 +- src/mongo/db/s/database_sharding_state.cpp | 57 ++++++++++++++-------- src/mongo/db/s/database_sharding_state.h | 52 ++++++++++++++------ src/mongo/db/s/database_sharding_state_test.cpp | 8 +-- src/mongo/db/s/drop_database_coordinator.cpp | 11 ++--- .../db/s/flush_database_cache_updates_command.cpp | 4 +- src/mongo/db/s/get_database_version_command.cpp | 4 +- src/mongo/db/s/move_primary_coordinator.cpp | 9 ++-- src/mongo/db/s/move_primary_source_manager.cpp | 20 ++++---- src/mongo/db/s/op_observer_sharding_impl.cpp | 4 +- src/mongo/db/s/op_observer_sharding_test.cpp | 4 +- .../db/s/shard_filtering_metadata_refresh.cpp | 27 +++++----- src/mongo/db/s/shard_server_op_observer.cpp | 19 ++++---- src/mongo/db/s/sharding_recovery_service.cpp | 7 ++- src/mongo/db/s/sharding_recovery_service_test.cpp | 24 ++++----- ..._move_primary_exit_critical_section_command.cpp | 4 +- src/mongo/db/shard_role_test.cpp | 21 ++++---- 20 files changed, 157 insertions(+), 134 deletions(-) diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 081c1150e94..05865883a13 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -75,8 +75,8 @@ MONGO_FAIL_POINT_DEFINE(hangAfterCollModIndexUniqueReleaseIXLock); void assertNoMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { try { - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, nss.dbName()); auto scopedCss = CollectionShardingState::assertCollectionLockedAndAcquire(opCtx, nss); auto collDesc = scopedCss->getCollectionDescription(opCtx); diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index 2aa9c668850..ed157697fa9 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -116,8 +116,8 @@ Status validateDBNameForWindows(StringData dbname) { } void assertNoMovePrimaryInProgress(OperationContext* opCtx, NamespaceString const& nss) { - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, nss.dbName()); if (scopedDss->isMovePrimaryInProgress()) { LOGV2(4909100, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp index ee0201f5f3e..5b207667fcf 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -365,8 +365,8 @@ void dropReadyIndexes(OperationContext* opCtx, void assertNoMovePrimaryInProgress(OperationContext* opCtx, const NamespaceString& nss) { try { - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, nss.dbName()); auto scopedCss = CollectionShardingState::assertCollectionLockedAndAcquire(opCtx, nss); auto collDesc = scopedCss->getCollectionDescription(opCtx); diff --git a/src/mongo/db/commands/create_indexes_cmd.cpp b/src/mongo/db/commands/create_indexes_cmd.cpp index 1b799c70c83..51e7007e5b9 100644 --- a/src/mongo/db/commands/create_indexes_cmd.cpp +++ b/src/mongo/db/commands/create_indexes_cmd.cpp @@ -317,8 +317,8 @@ bool indexesAlreadyExist(OperationContext* opCtx, void assertNoMovePrimaryInProgress(OperationContext* opCtx, const NamespaceString& nss) { try { - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, nss.dbName()); Lock::CollectionLock collLock(opCtx, nss, MODE_IX); diff --git a/src/mongo/db/s/database_sharding_state.cpp b/src/mongo/db/s/database_sharding_state.cpp index 74351ce4336..67a6f306f27 100644 --- a/src/mongo/db/s/database_sharding_state.cpp +++ b/src/mongo/db/s/database_sharding_state.cpp @@ -98,41 +98,56 @@ const ServiceContext::Decoration DatabaseShardingState } // namespace -DatabaseShardingState::ScopedDatabaseShardingState::ScopedDatabaseShardingState( +DatabaseShardingState::DatabaseShardingState(const DatabaseName& dbName) : _dbName(dbName) {} + +DatabaseShardingState::ScopedExclusiveDatabaseShardingState::ScopedExclusiveDatabaseShardingState( Lock::ResourceLock lock, DatabaseShardingState* dss) : _lock(std::move(lock)), _dss(dss) {} +DatabaseShardingState::ScopedSharedDatabaseShardingState::ScopedSharedDatabaseShardingState( + Lock::ResourceLock lock, DatabaseShardingState* dss) + : DatabaseShardingState::ScopedExclusiveDatabaseShardingState(std::move(lock), dss) {} -DatabaseShardingState::ScopedDatabaseShardingState::ScopedDatabaseShardingState( - ScopedDatabaseShardingState&& other) - : _lock(std::move(other._lock)), _dss(other._dss) { - other._dss = nullptr; -} - -DatabaseShardingState::ScopedDatabaseShardingState::~ScopedDatabaseShardingState() = default; +DatabaseShardingState::ScopedExclusiveDatabaseShardingState DatabaseShardingState::acquireExclusive( + OperationContext* opCtx, const DatabaseName& dbName) { -DatabaseShardingState::DatabaseShardingState(const DatabaseName& dbName) : _dbName(dbName) {} + DatabaseShardingStateMap::DSSAndLock* dssAndLock = + DatabaseShardingStateMap::get(opCtx->getServiceContext()).getOrCreate(dbName); -DatabaseShardingState::ScopedDatabaseShardingState DatabaseShardingState::assertDbLockedAndAcquire( - OperationContext* opCtx, const DatabaseName& dbName, DSSAcquisitionMode mode) { - dassert(opCtx->lockState()->isDbLockedForMode(dbName, MODE_IS)); + // First lock the RESOURCE_MUTEX associated to this dbName to guarantee stability of the + // DatabaseShardingState pointer. After that, it is safe to get and store the + // DatabaseShadingState*, as long as the RESOURCE_MUTEX is kept locked. + Lock::ResourceLock lock(opCtx->lockState(), dssAndLock->dssMutex.getRid(), MODE_X); - return acquire(opCtx, dbName, mode); + return ScopedExclusiveDatabaseShardingState(std::move(lock), dssAndLock->dss.get()); } -DatabaseShardingState::ScopedDatabaseShardingState DatabaseShardingState::acquire( - OperationContext* opCtx, const DatabaseName& dbName, DSSAcquisitionMode mode) { +DatabaseShardingState::ScopedSharedDatabaseShardingState DatabaseShardingState::acquireShared( + OperationContext* opCtx, const DatabaseName& dbName) { + DatabaseShardingStateMap::DSSAndLock* dssAndLock = DatabaseShardingStateMap::get(opCtx->getServiceContext()).getOrCreate(dbName); // First lock the RESOURCE_MUTEX associated to this dbName to guarantee stability of the // DatabaseShardingState pointer. After that, it is safe to get and store the // DatabaseShadingState*, as long as the RESOURCE_MUTEX is kept locked. - Lock::ResourceLock lock(opCtx->lockState(), - dssAndLock->dssMutex.getRid(), - mode == DSSAcquisitionMode::kShared ? MODE_IS : MODE_X); + Lock::ResourceLock lock(opCtx->lockState(), dssAndLock->dssMutex.getRid(), MODE_IS); + + return ScopedSharedDatabaseShardingState(std::move(lock), dssAndLock->dss.get()); +} - return ScopedDatabaseShardingState(std::move(lock), dssAndLock->dss.get()); +DatabaseShardingState::ScopedExclusiveDatabaseShardingState +DatabaseShardingState::assertDbLockedAndAcquireExclusive(OperationContext* opCtx, + const DatabaseName& dbName) { + dassert(opCtx->lockState()->isDbLockedForMode(dbName, MODE_IS)); + return acquireExclusive(opCtx, dbName); +} + +DatabaseShardingState::ScopedSharedDatabaseShardingState +DatabaseShardingState::assertDbLockedAndAcquireShared(OperationContext* opCtx, + const DatabaseName& dbName) { + dassert(opCtx->lockState()->isDbLockedForMode(dbName, MODE_IS)); + return acquireShared(opCtx, dbName); } std::vector DatabaseShardingState::getDatabaseNames(OperationContext* opCtx) { @@ -153,7 +168,7 @@ void DatabaseShardingState::assertMatchingDbVersion(OperationContext* opCtx, void DatabaseShardingState::assertMatchingDbVersion(OperationContext* opCtx, const DatabaseName& dbName, const DatabaseVersion& receivedVersion) { - const auto scopedDss = acquire(opCtx, dbName, DSSAcquisitionMode::kShared); + const auto scopedDss = acquireShared(opCtx, dbName); { const auto critSecSignal = scopedDss->getCriticalSectionSignal( @@ -192,7 +207,7 @@ void DatabaseShardingState::assertIsPrimaryShardForDb(OperationContext* opCtx, Lock::DBLock dbLock(opCtx, dbName, MODE_IS); assertMatchingDbVersion(opCtx, dbName); - const auto scopedDss = assertDbLockedAndAcquire(opCtx, dbName, DSSAcquisitionMode::kShared); + const auto scopedDss = assertDbLockedAndAcquireShared(opCtx, dbName); const auto primaryShardId = scopedDss->_dbInfo->getPrimary(); const auto thisShardId = ShardingState::get(opCtx)->shardId(); uassert(ErrorCodes::IllegalOperation, diff --git a/src/mongo/db/s/database_sharding_state.h b/src/mongo/db/s/database_sharding_state.h index ccc45f870c6..7824b1853d9 100644 --- a/src/mongo/db/s/database_sharding_state.h +++ b/src/mongo/db/s/database_sharding_state.h @@ -36,8 +36,6 @@ namespace mongo { -enum class DSSAcquisitionMode { kShared, kExclusive }; - /** * Synchronizes access to this shard server's cached database version for Database. */ @@ -50,18 +48,15 @@ public: DatabaseShardingState& operator=(const DatabaseShardingState&) = delete; /** - * 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. + * Obtains the sharding state for the specified database along with a resource lock in exclusive + * mode, which will be held until the object goes out of scope. */ - class ScopedDatabaseShardingState { + class ScopedExclusiveDatabaseShardingState { public: - ScopedDatabaseShardingState(ScopedDatabaseShardingState&&); - - ~ScopedDatabaseShardingState(); - DatabaseShardingState* operator->() const { return _dss; } + DatabaseShardingState& operator*() const { return *_dss; } @@ -69,18 +64,43 @@ public: private: friend class DatabaseShardingState; - ScopedDatabaseShardingState(Lock::ResourceLock lock, DatabaseShardingState* dss); + ScopedExclusiveDatabaseShardingState(Lock::ResourceLock lock, DatabaseShardingState* dss); 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); + /** + * Obtains the sharding state for the specified database along with a resource lock in shared + * mode, which will be held until the object goes out of scope. + */ + class ScopedSharedDatabaseShardingState : public ScopedExclusiveDatabaseShardingState { + public: + const DatabaseShardingState* operator->() const { + return _dss; + } + + const DatabaseShardingState& operator*() const { + return *_dss; + } + + private: + friend class DatabaseShardingState; + + ScopedSharedDatabaseShardingState(Lock::ResourceLock lock, DatabaseShardingState* dss); + }; + + static ScopedExclusiveDatabaseShardingState acquireExclusive(OperationContext* opCtx, + const DatabaseName& dbName); + + static ScopedSharedDatabaseShardingState acquireShared(OperationContext* opCtx, + const DatabaseName& dbName); + + static ScopedExclusiveDatabaseShardingState assertDbLockedAndAcquireExclusive( + OperationContext* opCtx, const DatabaseName& dbName); + + static ScopedSharedDatabaseShardingState assertDbLockedAndAcquireShared( + OperationContext* opCtx, const DatabaseName& dbName); /** * Returns the names of the databases that have a DatabaseShardingState. diff --git a/src/mongo/db/s/database_sharding_state_test.cpp b/src/mongo/db/s/database_sharding_state_test.cpp index 1ee64260111..b37f16c4212 100644 --- a/src/mongo/db/s/database_sharding_state_test.cpp +++ b/src/mongo/db/s/database_sharding_state_test.cpp @@ -136,8 +136,8 @@ TEST_F(DatabaseShardingStateTestWithMockedLoader, OnDbVersionMismatch) { auto getActiveDbVersion = [&] { AutoGetDb autoDb(opCtx, DatabaseName(boost::none, kDbName), MODE_IS); - const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, DatabaseName(boost::none, kDbName), DSSAcquisitionMode::kShared); + const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireShared( + opCtx, DatabaseName(boost::none, kDbName)); return scopedDss->getDbVersion(opCtx); }; @@ -171,8 +171,8 @@ TEST_F(DatabaseShardingStateTestWithMockedLoader, ForceDatabaseRefresh) { boost::optional activeDbVersion = [&] { AutoGetDb autoDb(opCtx, DatabaseName(boost::none, kDbName), MODE_IS); - const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, DatabaseName(boost::none, kDbName), DSSAcquisitionMode::kShared); + const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireShared( + opCtx, DatabaseName(boost::none, kDbName)); return scopedDss->getDbVersion(opCtx); }(); ASSERT_TRUE(activeDbVersion); diff --git a/src/mongo/db/s/drop_database_coordinator.cpp b/src/mongo/db/s/drop_database_coordinator.cpp index a2180bfac9f..5f9c6ba3cb9 100644 --- a/src/mongo/db/s/drop_database_coordinator.cpp +++ b/src/mongo/db/s/drop_database_coordinator.cpp @@ -158,8 +158,8 @@ public: // directly DatabaseName databaseName(boost::none, _dbName); Lock::DBLock dbLock(_opCtx, databaseName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - _opCtx, databaseName, DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(_opCtx, databaseName); scopedDss->enterCriticalSectionCatchUpPhase(_opCtx, _reason); scopedDss->enterCriticalSectionCommitPhase(_opCtx, _reason); } @@ -171,8 +171,8 @@ public: // directly DatabaseName databaseName(boost::none, _dbName); Lock::DBLock dbLock(_opCtx, databaseName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - _opCtx, databaseName, DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(_opCtx, databaseName); scopedDss->exitCriticalSection(_opCtx, _reason); } @@ -247,8 +247,7 @@ void DropDatabaseCoordinator::_dropShardedCollection( void DropDatabaseCoordinator::_clearDatabaseInfoOnPrimary(OperationContext* opCtx) { DatabaseName dbName(boost::none, _dbName); AutoGetDb autoDb(opCtx, dbName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName); scopedDss->clearDbInfo(opCtx); } 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 0fc2ec08d38..d6babcbbf6c 100644 --- a/src/mongo/db/s/flush_database_cache_updates_command.cpp +++ b/src/mongo/db/s/flush_database_cache_updates_command.cpp @@ -173,8 +173,8 @@ 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 scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, ns().dbName(), DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, ns().dbName()); criticalSectionSignal = scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite); } diff --git a/src/mongo/db/s/get_database_version_command.cpp b/src/mongo/db/s/get_database_version_command.cpp index 9cc375418da..6d23fc2782d 100644 --- a/src/mongo/db/s/get_database_version_command.cpp +++ b/src/mongo/db/s/get_database_version_command.cpp @@ -81,8 +81,8 @@ public: DatabaseName dbName(boost::none, _targetDb()); AutoGetDb autoDb(opCtx, dbName, MODE_IS); - const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, dbName); BSONObj versionObj; if (const auto dbVersion = scopedDss->getDbVersion(opCtx)) { diff --git a/src/mongo/db/s/move_primary_coordinator.cpp b/src/mongo/db/s/move_primary_coordinator.cpp index 9af0e59d141..e6fb1b958ae 100644 --- a/src/mongo/db/s/move_primary_coordinator.cpp +++ b/src/mongo/db/s/move_primary_coordinator.cpp @@ -461,8 +461,7 @@ void MovePrimaryCoordinator::assertChangedMetadataOnConfig( void MovePrimaryCoordinator::clearDbMetadataOnPrimary(OperationContext* opCtx) const { AutoGetDb autoDb(opCtx, _dbName, MODE_IX); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, _dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, _dbName); scopedDss->clearDbInfo(opCtx); } @@ -511,15 +510,13 @@ void MovePrimaryCoordinator::dropOrphanedDataOnRecipient( void MovePrimaryCoordinator::blockWritesLegacy(OperationContext* opCtx) const { AutoGetDb autoDb(opCtx, _dbName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, _dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, _dbName); scopedDss->setMovePrimaryInProgress(opCtx); } void MovePrimaryCoordinator::unblockWritesLegacy(OperationContext* opCtx) const { AutoGetDb autoDb(opCtx, _dbName, MODE_IX); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, _dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, _dbName); scopedDss->unsetMovePrimaryInProgress(opCtx); } diff --git a/src/mongo/db/s/move_primary_source_manager.cpp b/src/mongo/db/s/move_primary_source_manager.cpp index b8442b36f1c..e21cb688f27 100644 --- a/src/mongo/db/s/move_primary_source_manager.cpp +++ b/src/mongo/db/s/move_primary_source_manager.cpp @@ -106,8 +106,8 @@ Status MovePrimarySourceManager::clone(OperationContext* opCtx) { AutoGetDb autoDb(opCtx, getNss().dbName(), MODE_X); invariant(autoDb.ensureDbExists(opCtx), getNss().toString()); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, getNss().dbName()); scopedDss->setMovePrimaryInProgress(opCtx); } @@ -175,8 +175,8 @@ Status MovePrimarySourceManager::enterCriticalSection(OperationContext* opCtx) { << " was dropped during the movePrimary operation."); } - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, getNss().dbName()); // IMPORTANT: After this line, the critical section is in place and needs to be signaled scopedDss->enterCriticalSectionCatchUpPhase(opCtx, _critSecReason); @@ -226,8 +226,8 @@ Status MovePrimarySourceManager::commitOnConfig(OperationContext* opCtx) { << " was dropped during the movePrimary operation."); } - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, getNss().dbName()); // Read operations must begin to wait on the critical section just before we send the // commit operation to the config server @@ -287,8 +287,8 @@ Status MovePrimarySourceManager::commitOnConfig(OperationContext* opCtx) { } if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, getNss())) { - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive( + opCtx, getNss().dbName()); scopedDss->clearDbInfo(opCtx); uassertStatusOK(validateStatus.withContext( str::stream() << "Unable to verify movePrimary commit for database: " @@ -515,8 +515,8 @@ void MovePrimarySourceManager::_cleanup(OperationContext* opCtx) { UninterruptibleLockGuard noInterrupt(opCtx->lockState()); // NOLINT. AutoGetDb autoDb(opCtx, getNss().dbName(), MODE_IX); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, getNss().dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, getNss().dbName()); scopedDss->unsetMovePrimaryInProgress(opCtx); scopedDss->clearDbInfo(opCtx); diff --git a/src/mongo/db/s/op_observer_sharding_impl.cpp b/src/mongo/db/s/op_observer_sharding_impl.cpp index 9b01b2b24e3..c0a419f1dc5 100644 --- a/src/mongo/db/s/op_observer_sharding_impl.cpp +++ b/src/mongo/db/s/op_observer_sharding_impl.cpp @@ -76,8 +76,8 @@ void assertNoMovePrimaryInProgress(OperationContext* opCtx, const NamespaceStrin AllowLockAcquisitionOnTimestampedUnitOfWork allowLockAcquisition(opCtx->lockState()); Lock::DBLock dblock(opCtx, nss.dbName(), MODE_IS); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, nss.dbName(), DSSAcquisitionMode::kShared); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, nss.dbName()); if (scopedDss->isMovePrimaryInProgress()) { LOGV2(4908600, "assertNoMovePrimaryInProgress", "namespace"_attr = nss.toString()); diff --git a/src/mongo/db/s/op_observer_sharding_test.cpp b/src/mongo/db/s/op_observer_sharding_test.cpp index 1b5f2ee340d..a4676bd8c46 100644 --- a/src/mongo/db/s/op_observer_sharding_test.cpp +++ b/src/mongo/db/s/op_observer_sharding_test.cpp @@ -67,8 +67,8 @@ protected: bool justCreated = false; auto databaseHolder = DatabaseHolder::get(operationContext()); auto db = databaseHolder->openDb(operationContext(), kTestNss.dbName(), &justCreated); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - operationContext(), kTestNss.dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive( + operationContext(), kTestNss.dbName()); scopedDss->setDbInfo(operationContext(), DatabaseType{kTestNss.dbName().db(), ShardId("this"), dbVersion1}); ASSERT_TRUE(db); diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp index 5993369024d..7c1ac46bb09 100644 --- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp +++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp @@ -63,10 +63,10 @@ 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, - boost::optional* dbLock, - boost::optional* scopedDss) { +template +bool joinDbVersionOperation(OperationContext* opCtx, + boost::optional* dbLock, + boost::optional* scopedDss) { invariant(dbLock->has_value()); invariant(scopedDss->has_value()); @@ -122,8 +122,7 @@ Status refreshDbMetadata(OperationContext* opCtx, // TODO (SERVER-71444): Fix to be interruptible or document exception. // Can be uninterruptible because the work done under it can never block. UninterruptibleLockGuard noInterrupt(opCtx->lockState()); // NOLINT. - auto scopedDss = - DatabaseShardingState::acquire(opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::acquireExclusive(opCtx, dbName); scopedDss->resetDbMetadataRefreshFuture(); }); @@ -136,8 +135,7 @@ Status refreshDbMetadata(OperationContext* opCtx, // the number of possible threads convoying on the exclusive lock below. { Lock::DBLock dbLock(opCtx, dbName, MODE_IS); - const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kShared); + const auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, dbName); const auto cachedDbVersion = scopedDss->getDbVersion(opCtx); if (swDbMetadata.isOK() && swDbMetadata.getValue()->getVersion() <= cachedDbVersion) { @@ -153,8 +151,7 @@ Status refreshDbMetadata(OperationContext* opCtx, } Lock::DBLock dbLock(opCtx, dbName, MODE_IX); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName); if (!cancellationToken.isCanceled()) { if (swDbMetadata.isOK()) { // Set the refreshed database metadata in the local catalog. @@ -246,9 +243,8 @@ void onDbVersionMismatch(OperationContext* opCtx, dbLock.emplace(opCtx, dbName, MODE_IS); if (receivedDbVersion) { - boost::optional scopedDss( - DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kShared)); + auto scopedDss = boost::make_optional( + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx, dbName)); if (joinDbVersionOperation(opCtx, &dbLock, &scopedDss)) { // Waited for another thread to exit from the critical section or to complete an @@ -273,9 +269,8 @@ void onDbVersionMismatch(OperationContext* opCtx, return; } - boost::optional scopedDss( - DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive)); + auto scopedDss = boost::make_optional( + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName)); if (joinDbVersionOperation(opCtx, &dbLock, &scopedDss)) { // Waited for another thread to exit from the critical section or to complete an diff --git a/src/mongo/db/s/shard_server_op_observer.cpp b/src/mongo/db/s/shard_server_op_observer.cpp index b3f7c68ea88..d75ebd9b185 100644 --- a/src/mongo/db/s/shard_server_op_observer.cpp +++ b/src/mongo/db/s/shard_server_op_observer.cpp @@ -300,8 +300,8 @@ void ShardServerOpObserver::onInserts(OperationContext* opCtx, } // TODO (SERVER-71444): Fix to be interruptible or document exception. UninterruptibleLockGuard noInterrupt(opCtx->lockState()); // NOLINT. - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, insertedNss.dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive( + opCtx, insertedNss.dbName()); scopedDss->enterCriticalSectionCatchUpPhase(opCtx, reason); } else { boost::optional lockCollectionIfNotPrimary; @@ -433,8 +433,8 @@ void ShardServerOpObserver::onUpdate(OperationContext* opCtx, const OplogUpdateE DatabaseName dbName(boost::none, db); AutoGetDb autoDb(opCtx, dbName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName); scopedDss->clearDbInfo(opCtx); } } @@ -476,8 +476,8 @@ void ShardServerOpObserver::onUpdate(OperationContext* opCtx, const OplogUpdateE // TODO (SERVER-71444): Fix to be interruptible or document exception. UninterruptibleLockGuard noInterrupt(opCtx->lockState()); // NOLINT. - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, updatedNss.dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive( + opCtx, updatedNss.dbName()); scopedDss->enterCriticalSectionCommitPhase(opCtx, reason); } else { boost::optional lockCollectionIfNotPrimary; @@ -660,8 +660,7 @@ void ShardServerOpObserver::onDelete(OperationContext* opCtx, DatabaseName dbName(boost::none, deletedDatabase); AutoGetDb autoDb(opCtx, dbName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName); scopedDss->clearDbInfo(opCtx); } @@ -699,8 +698,8 @@ void ShardServerOpObserver::onDelete(OperationContext* opCtx, // TODO (SERVER-71444): Fix to be interruptible or document exception. UninterruptibleLockGuard noInterrupt(opCtx->lockState()); // NOLINT. - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, deletedNss.dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive( + opCtx, deletedNss.dbName()); // Secondary nodes must clear the database metadata before releasing the // in-memory critical section. diff --git a/src/mongo/db/s/sharding_recovery_service.cpp b/src/mongo/db/s/sharding_recovery_service.cpp index 38469c1f34b..1e30d725c9c 100644 --- a/src/mongo/db/s/sharding_recovery_service.cpp +++ b/src/mongo/db/s/sharding_recovery_service.cpp @@ -508,8 +508,7 @@ void ShardingRecoveryService::recoverRecoverableCriticalSections(OperationContex } for (const auto& dbName : DatabaseShardingState::getDatabaseNames(opCtx)) { AutoGetDb dbLock(opCtx, dbName, MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName); scopedDss->exitCriticalSectionNoChecks(opCtx); } @@ -521,8 +520,8 @@ void ShardingRecoveryService::recoverRecoverableCriticalSections(OperationContex { if (nsIsDbOnly(nss.ns())) { AutoGetDb dbLock(opCtx, nss.dbName(), MODE_X); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, nss.dbName(), DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, nss.dbName()); scopedDss->enterCriticalSectionCatchUpPhase(opCtx, doc.getReason()); if (doc.getBlockReads()) { scopedDss->enterCriticalSectionCommitPhase(opCtx, doc.getReason()); diff --git a/src/mongo/db/s/sharding_recovery_service_test.cpp b/src/mongo/db/s/sharding_recovery_service_test.cpp index 8996648cb0f..7b215873cf1 100644 --- a/src/mongo/db/s/sharding_recovery_service_test.cpp +++ b/src/mongo/db/s/sharding_recovery_service_test.cpp @@ -129,10 +129,10 @@ public: void assertCriticalSectionCatchUpEnteredInMemory(const NamespaceString& nss) { if (nsIsDbOnly(nss.ns())) { AutoGetDb db(opCtx(), nss.dbName(), MODE_IS); - auto dss = - DatabaseShardingState::acquire(opCtx(), nss.dbName(), DSSAcquisitionMode::kShared); - ASSERT(dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite)); - ASSERT(!dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx(), nss.dbName()); + ASSERT(scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite)); + ASSERT(!scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)); } else { AutoGetCollection coll(opCtx(), nss, MODE_IS); const auto csr = @@ -147,10 +147,10 @@ public: void assertCriticalSectionCommitEnteredInMemory(const NamespaceString& nss) { if (nsIsDbOnly(nss.ns())) { AutoGetDb db(opCtx(), nss.dbName(), MODE_IS); - auto dss = - DatabaseShardingState::acquire(opCtx(), nss.dbName(), DSSAcquisitionMode::kShared); - ASSERT(dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite)); - ASSERT(dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx(), nss.dbName()); + ASSERT(scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite)); + ASSERT(scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)); } else { AutoGetCollection coll(opCtx(), nss, MODE_IS); const auto csr = @@ -164,10 +164,10 @@ public: void assertCriticalSectionLeftInMemory(const NamespaceString& nss) { if (nsIsDbOnly(nss.ns())) { AutoGetDb db(opCtx(), nss.dbName(), MODE_IS); - auto dss = - DatabaseShardingState::acquire(opCtx(), nss.dbName(), DSSAcquisitionMode::kShared); - ASSERT(!dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite)); - ASSERT(!dss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)); + const auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireShared(opCtx(), nss.dbName()); + ASSERT(!scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kWrite)); + ASSERT(!scopedDss->getCriticalSectionSignal(ShardingMigrationCriticalSection::kRead)); } else { AutoGetCollection coll(opCtx(), nss, MODE_IS); const auto csr = diff --git a/src/mongo/db/s/shardsvr_move_primary_exit_critical_section_command.cpp b/src/mongo/db/s/shardsvr_move_primary_exit_critical_section_command.cpp index 6c972a6a27a..603f1b89fb7 100644 --- a/src/mongo/db/s/shardsvr_move_primary_exit_critical_section_command.cpp +++ b/src/mongo/db/s/shardsvr_move_primary_exit_critical_section_command.cpp @@ -86,8 +86,8 @@ public: { // Clear database metadata on primary node. AutoGetDb autoDb(newOpCtx.get(), dbName, MODE_IX); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - newOpCtx.get(), dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive( + newOpCtx.get(), dbName); scopedDss->clearDbInfo(newOpCtx.get()); } diff --git a/src/mongo/db/shard_role_test.cpp b/src/mongo/db/shard_role_test.cpp index 6d625a095f4..3dc175b6f8f 100644 --- a/src/mongo/db/shard_role_test.cpp +++ b/src/mongo/db/shard_role_test.cpp @@ -50,8 +50,7 @@ void installDatabaseMetadata(OperationContext* opCtx, const DatabaseName& dbName, const DatabaseVersion& dbVersion) { AutoGetDb autoDb(opCtx, dbName, MODE_X, {}); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx, dbName, DSSAcquisitionMode::kExclusive); + auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx, dbName); scopedDss->setDbInfo(opCtx, {dbName.db(), ShardId("this"), dbVersion}); } @@ -290,8 +289,8 @@ TEST_F(ShardRoleTest, AcquireUnshardedCollWhenShardDoesNotKnowThePlacementVersio { // Clear the database metadata AutoGetDb autoDb(opCtx(), dbNameTestDb, MODE_X, {}); - auto scopedDss = DatabaseShardingState::assertDbLockedAndAcquire( - opCtx(), dbNameTestDb, DSSAcquisitionMode::kExclusive); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx(), dbNameTestDb); scopedDss->clearDbInfo(opCtx()); } @@ -320,10 +319,10 @@ TEST_F(ShardRoleTest, AcquireUnshardedCollWhenCriticalSectionIsActiveThrows) { { // Enter critical section. AutoGetDb autoDb(opCtx(), dbNameTestDb, MODE_X, {}); - const auto dss = - DatabaseShardingState::acquire(opCtx(), dbNameTestDb, DSSAcquisitionMode::kExclusive); - dss->enterCriticalSectionCatchUpPhase(opCtx(), criticalSectionReason); - dss->enterCriticalSectionCommitPhase(opCtx(), criticalSectionReason); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx(), dbNameTestDb); + scopedDss->enterCriticalSectionCatchUpPhase(opCtx(), criticalSectionReason); + scopedDss->enterCriticalSectionCommitPhase(opCtx(), criticalSectionReason); } { @@ -351,9 +350,9 @@ TEST_F(ShardRoleTest, AcquireUnshardedCollWhenCriticalSectionIsActiveThrows) { // Exit critical section. AutoGetDb autoDb(opCtx(), dbNameTestDb, MODE_X, {}); const BSONObj criticalSectionReason = BSON("reason" << 1); - const auto dss = - DatabaseShardingState::acquire(opCtx(), dbNameTestDb, DSSAcquisitionMode::kExclusive); - dss->exitCriticalSection(opCtx(), criticalSectionReason); + auto scopedDss = + DatabaseShardingState::assertDbLockedAndAcquireExclusive(opCtx(), dbNameTestDb); + scopedDss->exitCriticalSection(opCtx(), criticalSectionReason); } } -- cgit v1.2.1