diff options
-rw-r--r-- | src/mongo/db/index_builds_coordinator.cpp | 110 | ||||
-rw-r--r-- | src/mongo/db/index_builds_coordinator.h | 126 | ||||
-rw-r--r-- | src/mongo/db/index_builds_coordinator_mongod_test.cpp | 172 |
3 files changed, 39 insertions, 369 deletions
diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 3b524dea8c7..fafdc30c716 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -340,8 +340,6 @@ IndexBuildsCoordinator* IndexBuildsCoordinator::get(OperationContext* operationC IndexBuildsCoordinator::~IndexBuildsCoordinator() { invariant(_databaseIndexBuilds.empty()); - invariant(_disallowedDbs.empty()); - invariant(_disallowedCollections.empty()); invariant(_collectionIndexBuilds.empty()); } @@ -549,25 +547,43 @@ std::vector<UUID> IndexBuildsCoordinator::abortCollectionIndexBuildsNoWait( return _abortCollectionIndexBuilds(lk, collectionUUID, reason, shouldWait); } -void IndexBuildsCoordinator::abortDatabaseIndexBuilds(StringData db, const std::string& reason) { - stdx::unique_lock<Latch> lk(_mutex); - - // Ensure the caller correctly stopped any new index builds on the database. - auto it = _disallowedDbs.find(db); - invariant(it != _disallowedDbs.end()); - +void IndexBuildsCoordinator::_abortDatabaseIndexBuilds(stdx::unique_lock<Latch>& lk, + const StringData& db, + const std::string& reason, + bool shouldWait) { auto dbIndexBuilds = _databaseIndexBuilds[db]; if (!dbIndexBuilds) { return; } + LOGV2(4612302, + "About to abort all index builders running for collections in the given database", + "database"_attr = db); + dbIndexBuilds->runOperationOnAllBuilds(lk, &_indexBuildsManager, abortIndexBuild, reason); + if (!shouldWait) { + return; + } + // 'dbIndexBuilds' is a shared ptr, so it can be safely waited upon without destructing before // waitUntilNoIndexBuildsRemain() returns, which would cause a use-after-free memory error. dbIndexBuilds->waitUntilNoIndexBuildsRemain(lk); } +void IndexBuildsCoordinator::abortDatabaseIndexBuilds(StringData db, const std::string& reason) { + stdx::unique_lock<Latch> lk(_mutex); + const bool shouldWait = true; + _abortDatabaseIndexBuilds(lk, db, reason, shouldWait); +} + +void IndexBuildsCoordinator::abortDatabaseIndexBuildsNoWait(StringData db, + const std::string& reason) { + stdx::unique_lock<Latch> lk(_mutex); + const bool shouldWait = false; + _abortDatabaseIndexBuilds(lk, db, reason, shouldWait); +} + namespace { NamespaceString getNsFromUUID(OperationContext* opCtx, const UUID& uuid) { auto& catalog = CollectionCatalog::get(opCtx); @@ -1133,8 +1149,6 @@ void IndexBuildsCoordinator::sleepIndexBuilds_forTestOnly(bool sleep) { void IndexBuildsCoordinator::verifyNoIndexBuilds_forTestOnly() { invariant(_databaseIndexBuilds.empty()); - invariant(_disallowedDbs.empty()); - invariant(_disallowedCollections.empty()); invariant(_collectionIndexBuilds.empty()); } @@ -1170,16 +1184,6 @@ void IndexBuildsCoordinator::updateCurOpOpDescription(OperationContext* opCtx, Status IndexBuildsCoordinator::_registerIndexBuild( WithLock lk, std::shared_ptr<ReplIndexBuildState> replIndexBuildState) { - - auto itns = _disallowedCollections.find(replIndexBuildState->collectionUUID); - auto itdb = _disallowedDbs.find(replIndexBuildState->dbName); - if (itns != _disallowedCollections.end() || itdb != _disallowedDbs.end()) { - return Status(ErrorCodes::CannotCreateIndex, - str::stream() << "Collection ( " << replIndexBuildState->collectionUUID - << " ) is in the process of being dropped. New index builds " - "are not currently allowed."); - } - // Check whether any indexes are already being built with the same index name(s). (Duplicate // specs will be discovered by the index builder.) auto collIndexBuildsIt = _collectionIndexBuilds.find(replIndexBuildState->collectionUUID); @@ -2159,50 +2163,6 @@ StatusWith<std::pair<long long, long long>> IndexBuildsCoordinator::_runIndexReb return status; } -void IndexBuildsCoordinator::_stopIndexBuildsOnDatabase(StringData dbName) { - stdx::unique_lock<Latch> lk(_mutex); - - auto it = _disallowedDbs.find(dbName); - if (it != _disallowedDbs.end()) { - ++(it->second); - return; - } - _disallowedDbs[dbName] = 1; -} - -void IndexBuildsCoordinator::_stopIndexBuildsOnCollection(const UUID& collectionUUID) { - stdx::unique_lock<Latch> lk(_mutex); - - auto it = _disallowedCollections.find(collectionUUID); - if (it != _disallowedCollections.end()) { - ++(it->second); - return; - } - _disallowedCollections[collectionUUID] = 1; -} - -void IndexBuildsCoordinator::_allowIndexBuildsOnDatabase(StringData dbName) { - stdx::unique_lock<Latch> lk(_mutex); - - auto it = _disallowedDbs.find(dbName); - invariant(it != _disallowedDbs.end()); - invariant(it->second); - if (--(it->second) == 0) { - _disallowedDbs.erase(it); - } -} - -void IndexBuildsCoordinator::_allowIndexBuildsOnCollection(const UUID& collectionUUID) { - stdx::unique_lock<Latch> lk(_mutex); - - auto it = _disallowedCollections.find(collectionUUID); - invariant(it != _disallowedCollections.end()); - invariant(it->second > 0); - if (--(it->second) == 0) { - _disallowedCollections.erase(it); - } -} - StatusWith<std::shared_ptr<ReplIndexBuildState>> IndexBuildsCoordinator::_getIndexBuild( const UUID& buildUUID) const { stdx::unique_lock<Latch> lk(_mutex); @@ -2224,26 +2184,6 @@ std::vector<std::shared_ptr<ReplIndexBuildState>> IndexBuildsCoordinator::_getIn return indexBuilds; } -ScopedStopNewDatabaseIndexBuilds::ScopedStopNewDatabaseIndexBuilds( - IndexBuildsCoordinator* indexBuildsCoordinator, StringData dbName) - : _indexBuildsCoordinatorPtr(indexBuildsCoordinator), _dbName(dbName.toString()) { - _indexBuildsCoordinatorPtr->_stopIndexBuildsOnDatabase(_dbName); -} - -ScopedStopNewDatabaseIndexBuilds::~ScopedStopNewDatabaseIndexBuilds() { - _indexBuildsCoordinatorPtr->_allowIndexBuildsOnDatabase(_dbName); -} - -ScopedStopNewCollectionIndexBuilds::ScopedStopNewCollectionIndexBuilds( - IndexBuildsCoordinator* indexBuildsCoordinator, const UUID& collectionUUID) - : _indexBuildsCoordinatorPtr(indexBuildsCoordinator), _collectionUUID(collectionUUID) { - _indexBuildsCoordinatorPtr->_stopIndexBuildsOnCollection(_collectionUUID); -} - -ScopedStopNewCollectionIndexBuilds::~ScopedStopNewCollectionIndexBuilds() { - _indexBuildsCoordinatorPtr->_allowIndexBuildsOnCollection(_collectionUUID); -} - int IndexBuildsCoordinator::getNumIndexesTotal(OperationContext* opCtx, Collection* collection) { invariant(collection); const auto& nss = collection->ns(); diff --git a/src/mongo/db/index_builds_coordinator.h b/src/mongo/db/index_builds_coordinator.h index 1be72d0554c..2e4c42d1ef8 100644 --- a/src/mongo/db/index_builds_coordinator.h +++ b/src/mongo/db/index_builds_coordinator.h @@ -197,16 +197,6 @@ public: * must continue to operate on the collection by UUID to protect against rename collection. The * provided 'reason' will be used in the error message that the index builders return to their * callers. - * - * First create a ScopedStopNewCollectionIndexBuilds to block further index builds on the - * collection before calling this and for the duration of the drop collection operation. - * - * { - * ScopedStopNewCollectionIndexBuilds scopedStop(collectionUUID); - * indexBuildsCoord->abortCollectionIndexBuilds(collectionUUID, "..."); - * AutoGetCollection autoColl(..., collectionUUID, ...); - * autoColl->dropCollection(...); - * } */ void abortCollectionIndexBuilds(const UUID& collectionUUID, const std::string& reason); @@ -223,21 +213,16 @@ public: * Signals all of the index builds on the specified 'db' to abort and then waits until the index * builds are no longer running. The provided 'reason' will be used in the error message that * the index builders return to their callers. - * - * First create a ScopedStopNewDatabaseIndexBuilds to block further index builds on the - * specified - * database before calling this and for the duration of the drop database operation. - * - * { - * ScopedStopNewDatabaseIndexBuilds scopedStop(dbName); - * indexBuildsCoord->abortDatabaseIndexBuilds(dbName, "..."); - * AutoGetDb autoDb(...); - * autoDb->dropDatabase(...); - * } */ void abortDatabaseIndexBuilds(StringData db, const std::string& reason); /** + * Signals all of the index builds on the specified database to abort. The provided 'reason' + * will be used in the error message that the index builders return to their callers. + */ + void abortDatabaseIndexBuildsNoWait(StringData db, const std::string& reason); + + /** * Aborts an index build by index build UUID. Returns when the index build thread exits. */ void abortIndexBuildByBuildUUID(OperationContext* opCtx, @@ -437,28 +422,6 @@ public: private: - // Friend classes in order to be the only allowed callers of - //_stopIndexBuildsOnCollection/Database and _allowIndexBuildsOnCollection/Database. - friend class ScopedStopNewDatabaseIndexBuilds; - friend class ScopedStopNewCollectionIndexBuilds; - - /** - * Prevents new index builds being registered on the provided collection or database. - * - * It is safe to call this on the same collection/database concurrently in different threads. It - * will still behave correctly. - */ - void _stopIndexBuildsOnDatabase(StringData dbName); - void _stopIndexBuildsOnCollection(const UUID& collectionUUID); - - /** - * Allows new index builds to again be registered on the provided collection or database. Should - * only be called after calling stopIndexBuildsOnCollection or stopIndexBuildsOnDatabase on the - * same collection or database, respectively. - */ - void _allowIndexBuildsOnDatabase(StringData dbName); - void _allowIndexBuildsOnCollection(const UUID& collectionUUID); - /** * Registers an index build so that the rest of the system can discover it. * @@ -673,16 +636,17 @@ protected: const std::string& reason, bool shouldWait); + /** + * Helper for 'abortDatabaseIndexBuilds' and 'abortDatabaseIndexBuildsNoWait'. + */ + void _abortDatabaseIndexBuilds(stdx::unique_lock<Latch>& lk, + const StringData& db, + const std::string& reason, + bool shouldWait); + // Protects the below state. mutable Mutex _mutex = MONGO_MAKE_LATCH("IndexBuildsCoordinator::_mutex"); - // New index builds are not allowed on a collection or database if the collection or database is - // in either of these maps. These are used when concurrent operations need to abort index builds - // on a collection or database and must wait for the index builds to drain, without further - // index builds being allowed to begin. - StringMap<int> _disallowedDbs; - stdx::unordered_map<UUID, int, UUID::Hash> _disallowedCollections; - // Maps database name to database information. Tracks and accesses index builds on a database // level. Can be used to abort and wait upon the completion of all index builds for a database. // @@ -709,68 +673,6 @@ protected: bool _sleepForTest = false; }; -/** - * For this object's lifetime no new index builds will be allowed on the specified database. An - * error will be returned by the IndexBuildsCoordinator to any caller attempting to register a new - * index build on the blocked collection or database. - * - * This should be used by operations like drop database, where the active index builds must be - * signaled to abort, but it takes time for them to wrap up, during which time no further index - * builds should be scheduled. - */ -class ScopedStopNewDatabaseIndexBuilds { - ScopedStopNewDatabaseIndexBuilds(const ScopedStopNewDatabaseIndexBuilds&) = delete; - ScopedStopNewDatabaseIndexBuilds& operator=(const ScopedStopNewDatabaseIndexBuilds&) = delete; - -public: - /** - * Takes either the full collection namespace or a database name and will block further index - * builds on that collection or database. - */ - ScopedStopNewDatabaseIndexBuilds(IndexBuildsCoordinator* indexBuildsCoordinator, - StringData dbName); - - /** - * Allows new index builds on the collection or database that were previously disallowed. - */ - ~ScopedStopNewDatabaseIndexBuilds(); - -private: - IndexBuildsCoordinator* _indexBuildsCoordinatorPtr; - std::string _dbName; -}; - -/** - * For this object's lifetime no new index builds will be allowed on the specified collection. An - * error will be returned by the IndexBuildsCoordinator to any caller attempting to register a new - * index build on the blocked collection. - * - * This should be used by operations like drop collection, where the active index builds must be - * signaled to abort, but it takes time for them to wrap up, during which time no further index - * builds should be scheduled. - */ -class ScopedStopNewCollectionIndexBuilds { - ScopedStopNewCollectionIndexBuilds(const ScopedStopNewCollectionIndexBuilds&) = delete; - ScopedStopNewCollectionIndexBuilds& operator=(const ScopedStopNewCollectionIndexBuilds&) = - delete; - -public: - /** - * Blocks further index builds on the specified collection. - */ - ScopedStopNewCollectionIndexBuilds(IndexBuildsCoordinator* indexBuildsCoordinator, - const UUID& collectionUUID); - - /** - * Allows new index builds on the collection that were previously disallowed. - */ - ~ScopedStopNewCollectionIndexBuilds(); - -private: - IndexBuildsCoordinator* _indexBuildsCoordinatorPtr; - UUID _collectionUUID; -}; - // These fail points are used to control index build progress. Declared here to be shared // temporarily between createIndexes command and IndexBuildsCoordinator. extern FailPoint hangAfterIndexBuildFirstDrain; diff --git a/src/mongo/db/index_builds_coordinator_mongod_test.cpp b/src/mongo/db/index_builds_coordinator_mongod_test.cpp index 8a9939579b4..8709041d43f 100644 --- a/src/mongo/db/index_builds_coordinator_mongod_test.cpp +++ b/src/mongo/db/index_builds_coordinator_mongod_test.cpp @@ -258,178 +258,6 @@ TEST_F(IndexBuildsCoordinatorMongodTest, Registration) { ASSERT_NOT_EQUALS(_testFooNss, _othertestFooNss); } -// Exercises the stopIndexBuildsOnCollection/Database() and allowIndexBuildsOnCollection/Database() -// functions, checking that they correctly disallow and allow index builds when -// ScopedStopNewCollectionIndexBuilds and ScopedStopNewDatabaseIndexBuilds are present on a -// collection or database name. -TEST_F(IndexBuildsCoordinatorMongodTest, DisallowNewBuildsOnNamespace) { - { - _indexBuildsCoord->sleepIndexBuilds_forTestOnly(true); - - // Create a scoped object to block new index builds ONLY on _testFooNss. - ScopedStopNewCollectionIndexBuilds scopedStop(_indexBuildsCoord.get(), _testFooUUID); - - // Registering an index build on _testFooNss should fail. - ASSERT_EQ(ErrorCodes::CannotCreateIndex, - _indexBuildsCoord - ->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"a", "b"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions) - .getStatus()); - - // Registering index builds on other collections and databases should still succeed. - auto testBarFuture = - assertGet(_indexBuildsCoord->startIndexBuild(operationContext(), - _testBarNss.db().toString(), - _testBarUUID, - makeSpecs(_testBarNss, {"c", "d"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions)); - auto othertestFooFuture = - assertGet(_indexBuildsCoord->startIndexBuild(operationContext(), - _othertestFooNss.db().toString(), - _othertestFooUUID, - makeSpecs(_othertestFooNss, {"e", "f"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions)); - - _indexBuildsCoord->sleepIndexBuilds_forTestOnly(false); - - auto indexCatalogStats = unittest::assertGet(testBarFuture.getNoThrow()); - ASSERT_EQ(1, indexCatalogStats.numIndexesBefore); - ASSERT_EQ(3, indexCatalogStats.numIndexesAfter); - indexCatalogStats = unittest::assertGet(othertestFooFuture.getNoThrow()); - ASSERT_EQ(1, indexCatalogStats.numIndexesBefore); - ASSERT_EQ(3, indexCatalogStats.numIndexesAfter); - } - - { - // Check that the scoped object correctly cleared. - auto testFooFuture = - assertGet(_indexBuildsCoord->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"a", "b"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions)); - auto indexCatalogStats = unittest::assertGet(testFooFuture.getNoThrow()); - ASSERT_EQ(1, indexCatalogStats.numIndexesBefore); - ASSERT_EQ(3, indexCatalogStats.numIndexesAfter); - } - - { - _indexBuildsCoord->sleepIndexBuilds_forTestOnly(true); - - // Create a scoped object to block new index builds on the 'test' database. - ScopedStopNewDatabaseIndexBuilds scopedStop(_indexBuildsCoord.get(), _testFooNss.db()); - - // Registering an index build on any collection in the 'test' database should fail. - ASSERT_EQ(ErrorCodes::CannotCreateIndex, - _indexBuildsCoord - ->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"c", "d"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions) - .getStatus()); - ASSERT_EQ(ErrorCodes::CannotCreateIndex, - _indexBuildsCoord - ->startIndexBuild(operationContext(), - _testBarNss.db().toString(), - _testBarUUID, - makeSpecs(_testBarNss, {"a", "b"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions) - .getStatus()); - - // Registering index builds on another database should still succeed. - auto othertestFooFuture = - assertGet(_indexBuildsCoord->startIndexBuild(operationContext(), - _othertestFooNss.db().toString(), - _othertestFooUUID, - makeSpecs(_othertestFooNss, {"g", "h"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions)); - - _indexBuildsCoord->sleepIndexBuilds_forTestOnly(false); - - auto indexCatalogStats = unittest::assertGet(othertestFooFuture.getNoThrow()); - ASSERT_EQ(3, indexCatalogStats.numIndexesBefore); - ASSERT_EQ(5, indexCatalogStats.numIndexesAfter); - } - - { - // Check that the scoped object correctly cleared. - auto testFooFuture = - assertGet(_indexBuildsCoord->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"c", "d"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions)); - auto indexCatalogStats = unittest::assertGet(testFooFuture.getNoThrow()); - ASSERT_EQ(3, indexCatalogStats.numIndexesBefore); - ASSERT_EQ(5, indexCatalogStats.numIndexesAfter); - } - - { - // Test concurrency of multiple scoped objects to block an index builds. - - ScopedStopNewCollectionIndexBuilds scopedStop(_indexBuildsCoord.get(), _testFooUUID); - { - ScopedStopNewCollectionIndexBuilds scopedStop(_indexBuildsCoord.get(), _testFooUUID); - - ASSERT_EQ(ErrorCodes::CannotCreateIndex, - _indexBuildsCoord - ->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"e", "f"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions) - .getStatus()); - } - ASSERT_EQ(ErrorCodes::CannotCreateIndex, - _indexBuildsCoord - ->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"e", "f"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions) - .getStatus()); - } - - { - // Check that the scoped object correctly cleared. - auto testFooFuture = - assertGet(_indexBuildsCoord->startIndexBuild(operationContext(), - _testFooNss.db().toString(), - _testFooUUID, - makeSpecs(_testFooNss, {"e", "f"}), - UUID::gen(), - IndexBuildProtocol::kTwoPhase, - _indexBuildOptions)); - auto indexCatalogStats = unittest::assertGet(testFooFuture.getNoThrow()); - ASSERT_EQ(5, indexCatalogStats.numIndexesBefore); - ASSERT_EQ(7, indexCatalogStats.numIndexesAfter); - } -} - TEST_F(IndexBuildsCoordinatorMongodTest, SetCommitQuorumWithBadArguments) { _indexBuildsCoord->sleepIndexBuilds_forTestOnly(true); |