From 83fa6e3879ab93549824fff82cab7030869563d0 Mon Sep 17 00:00:00 2001 From: Dianna Hohensee Date: Tue, 12 Mar 2019 09:53:51 -0400 Subject: Revert "SERVER-39079 Move BackgroundOperation checks out of the catalog layer; add parallel IndexBuildsCoordinator checks for all BackgroundOperation checks" This reverts commit d02edd5290131978f901ffc657bee3470d03f8fd. --- src/mongo/db/SConscript | 1 + src/mongo/db/background.cpp | 11 ----- src/mongo/db/background.h | 1 - src/mongo/db/catalog/SConscript | 3 -- src/mongo/db/catalog/capped_utils.cpp | 4 -- src/mongo/db/catalog/catalog_control.cpp | 5 --- src/mongo/db/catalog/catalog_control_test.cpp | 5 --- src/mongo/db/catalog/coll_mod.cpp | 5 --- src/mongo/db/catalog/collection.h | 6 --- src/mongo/db/catalog/collection_impl.cpp | 2 + src/mongo/db/catalog/collection_impl.h | 6 --- src/mongo/db/catalog/database.h | 3 -- src/mongo/db/catalog/database_holder.h | 5 +-- src/mongo/db/catalog/database_holder_impl.cpp | 15 +------ src/mongo/db/catalog/database_impl.cpp | 20 +++------- src/mongo/db/catalog/database_impl.h | 6 --- src/mongo/db/catalog/database_test.cpp | 4 +- src/mongo/db/catalog/drop_collection.cpp | 3 -- src/mongo/db/catalog/drop_database.cpp | 9 ----- src/mongo/db/catalog/drop_indexes.cpp | 6 +-- src/mongo/db/catalog/index_catalog.h | 11 ----- src/mongo/db/catalog/index_catalog_impl.cpp | 13 ++---- src/mongo/db/catalog/index_catalog_impl.h | 10 ----- src/mongo/db/catalog/index_catalog_noop.h | 4 -- src/mongo/db/catalog/rename_collection.cpp | 7 ---- src/mongo/db/commands/mr.cpp | 35 ++++++---------- src/mongo/db/commands/rename_collection_cmd.cpp | 8 ++++ src/mongo/db/commands/test_commands.cpp | 5 --- src/mongo/db/index_builds_coordinator.cpp | 53 ++++++++++++++----------- src/mongo/db/index_builds_coordinator.h | 12 +++--- src/mongo/db/repl/SConscript | 1 - src/mongo/db/repl/apply_ops.cpp | 7 ++-- src/mongo/db/repl/idempotency_test_fixture.cpp | 21 +++------- src/mongo/db/repl/idempotency_test_fixture.h | 4 +- src/mongo/db/repl/oplog.cpp | 13 +----- src/mongo/db/repl/rollback_impl.cpp | 8 +--- src/mongo/db/repl/sync_tail_test.cpp | 16 ++++---- 37 files changed, 94 insertions(+), 254 deletions(-) (limited to 'src') diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 0e87f6bd85b..a9389aaad96 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -990,6 +990,7 @@ env.Library( "query_exec", "service_context", "$BUILD_DIR/mongo/base", + "$BUILD_DIR/mongo/db/catalog/catalog_helpers", "$BUILD_DIR/mongo/db/catalog/commit_quorum_options", "$BUILD_DIR/mongo/db/catalog/index_build_entry_idl", "$BUILD_DIR/mongo/db/matcher/expressions_mongod_only", diff --git a/src/mongo/db/background.cpp b/src/mongo/db/background.cpp index f499e8a1a2e..eb24f3c6529 100644 --- a/src/mongo/db/background.cpp +++ b/src/mongo/db/background.cpp @@ -136,17 +136,6 @@ bool BackgroundOperation::inProgForNs(StringData ns) { return nsInProg.find(ns) != nsInProg.end(); } -void BackgroundOperation::assertNoBgOpInProg() { - for (auto& db : dbsInProg) { - uassert(ErrorCodes::BackgroundOperationInProgressForDatabase, - mongoutils::str::stream() - << "cannot perform operation: a background operation is currently running for " - "database " - << db.first, - !inProgForDb(db.first)); - } -} - void BackgroundOperation::assertNoBgOpInProgForDb(StringData db) { uassert(ErrorCodes::BackgroundOperationInProgressForDatabase, mongoutils::str::stream() diff --git a/src/mongo/db/background.h b/src/mongo/db/background.h index 6aa4a680e03..97495163b44 100644 --- a/src/mongo/db/background.h +++ b/src/mongo/db/background.h @@ -61,7 +61,6 @@ public: static bool inProgForDb(StringData db); static int numInProgForDb(StringData db); static bool inProgForNs(StringData ns); - static void assertNoBgOpInProg(); static void assertNoBgOpInProgForDb(StringData db); static void assertNoBgOpInProgForNs(StringData ns); static void awaitNoBgOpInProgForDb(StringData db); diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript index e8bc9168d3c..efc323713ce 100644 --- a/src/mongo/db/catalog/SConscript +++ b/src/mongo/db/catalog/SConscript @@ -355,7 +355,6 @@ env.Library( LIBDEPS_PRIVATE=[ 'collection', 'uuid_catalog', - '$BUILD_DIR/mongo/db/index_builds_coordinator_interface', '$BUILD_DIR/mongo/db/repair_database', '$BUILD_DIR/mongo/db/service_context', ], @@ -369,7 +368,6 @@ env.CppUnitTest( LIBDEPS=[ 'catalog_control', 'database_holder', - '$BUILD_DIR/mongo/db/index_builds_coordinator_mongod', '$BUILD_DIR/mongo/db/service_context', ], ) @@ -438,7 +436,6 @@ env.Library( '$BUILD_DIR/mongo/base', '$BUILD_DIR/mongo/db/background', '$BUILD_DIR/mongo/db/db_raii', - '$BUILD_DIR/mongo/db/index_builds_coordinator_interface', '$BUILD_DIR/mongo/db/query_exec', '$BUILD_DIR/mongo/db/server_options_core', '$BUILD_DIR/mongo/db/views/views', diff --git a/src/mongo/db/catalog/capped_utils.cpp b/src/mongo/db/catalog/capped_utils.cpp index 0f9562d653c..2ddae23aa80 100644 --- a/src/mongo/db/catalog/capped_utils.cpp +++ b/src/mongo/db/catalog/capped_utils.cpp @@ -45,7 +45,6 @@ #include "mongo/db/client.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/curop.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/op_observer.h" #include "mongo/db/query/internal_plans.h" #include "mongo/db/query/plan_yield_policy.h" @@ -96,8 +95,6 @@ Status emptyCapped(OperationContext* opCtx, const NamespaceString& collectionNam } BackgroundOperation::assertNoBgOpInProgForNs(collectionName.ns()); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - collection->uuid().get()); WriteUnitOfWork wuow(opCtx); @@ -261,7 +258,6 @@ void convertToCapped(OperationContext* opCtx, ErrorCodes::NamespaceNotFound, str::stream() << "database " << dbname << " not found", db); BackgroundOperation::assertNoBgOpInProgForDb(dbname); - IndexBuildsCoordinator::get(opCtx)->assertNoBgOpInProgForDb(dbname); // Generate a temporary collection name that will not collide with any existing collections. auto tmpNameResult = diff --git a/src/mongo/db/catalog/catalog_control.cpp b/src/mongo/db/catalog/catalog_control.cpp index f3e93ba6814..e9199519632 100644 --- a/src/mongo/db/catalog/catalog_control.cpp +++ b/src/mongo/db/catalog/catalog_control.cpp @@ -32,14 +32,12 @@ #include "mongo/db/catalog/catalog_control.h" -#include "mongo/db/background.h" #include "mongo/db/catalog/collection.h" #include "mongo/db/catalog/database.h" #include "mongo/db/catalog/database_catalog_entry.h" #include "mongo/db/catalog/database_holder.h" #include "mongo/db/catalog/uuid_catalog.h" #include "mongo/db/ftdc/ftdc_mongod.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/namespace_string.h" #include "mongo/db/repair_database.h" #include "mongo/util/log.h" @@ -49,9 +47,6 @@ namespace catalog { MinVisibleTimestampMap closeCatalog(OperationContext* opCtx) { invariant(opCtx->lockState()->isW()); - BackgroundOperation::assertNoBgOpInProg(); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgress(); - MinVisibleTimestampMap minVisibleTimestampMap; std::vector allDbs; opCtx->getServiceContext()->getStorageEngine()->listDatabases(&allDbs); diff --git a/src/mongo/db/catalog/catalog_control_test.cpp b/src/mongo/db/catalog/catalog_control_test.cpp index 39b4d74153b..b887b9ef5d7 100644 --- a/src/mongo/db/catalog/catalog_control_test.cpp +++ b/src/mongo/db/catalog/catalog_control_test.cpp @@ -31,7 +31,6 @@ #include "mongo/db/catalog/database_holder_mock.h" #include "mongo/db/client.h" -#include "mongo/db/index_builds_coordinator_mongod.h" #include "mongo/db/operation_context_noop.h" #include "mongo/db/service_context.h" #include "mongo/db/storage/storage_engine.h" @@ -113,10 +112,6 @@ void CatalogControlTest::setUp() { auto storageEngine = std::make_unique(); serviceContext->setStorageEngine(std::move(storageEngine)); DatabaseHolder::set(serviceContext.get(), std::make_unique()); - // Only need the IndexBuildsCoordinator to call into and check whether there are any index - // builds in progress. - IndexBuildsCoordinator::set(serviceContext.get(), - std::make_unique()); setGlobalServiceContext(std::move(serviceContext)); } diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 6e2ac01fb74..b28a7bbccdb 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -45,7 +45,6 @@ #include "mongo/db/command_generic_argument.h" #include "mongo/db/db_raii.h" #include "mongo/db/index/index_descriptor.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/op_observer.h" #include "mongo/db/repl/repl_client_info.h" #include "mongo/db/repl/replication_coordinator.h" @@ -320,10 +319,6 @@ Status _collModInternal(OperationContext* opCtx, // This can kill all cursors so don't allow running it while a background operation is in // progress. BackgroundOperation::assertNoBgOpInProgForNs(nss); - if (coll) { - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - coll->uuid().get()); - } // If db/collection/view does not exist, short circuit and return. if (!db || (!coll && !view)) { diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h index 85598d29f51..b32c0e449b8 100644 --- a/src/mongo/db/catalog/collection.h +++ b/src/mongo/db/catalog/collection.h @@ -315,9 +315,6 @@ public: * removes all documents as fast as possible * indexes before and after will be the same * as will other characteristics. - * - * The caller should hold a collection X lock and ensure there are no index builds in progress - * on the collection. */ virtual Status truncate(OperationContext* const opCtx) = 0; @@ -346,9 +343,6 @@ public: * collection. The collection cannot be completely emptied using this * function. An assertion will be thrown if that is attempted. * @param inclusive - Truncate 'end' as well iff true - * - * The caller should hold a collection X lock and ensure there are no index builds in progress - * on the collection. */ virtual void cappedTruncateAfter(OperationContext* const opCtx, RecordId end, diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp index 1d53ce43989..a11d2461ba1 100644 --- a/src/mongo/db/catalog/collection_impl.cpp +++ b/src/mongo/db/catalog/collection_impl.cpp @@ -780,6 +780,7 @@ uint64_t CollectionImpl::getIndexSize(OperationContext* opCtx, BSONObjBuilder* d */ Status CollectionImpl::truncate(OperationContext* opCtx) { dassert(opCtx->lockState()->isCollectionLockedForMode(ns(), MODE_X)); + BackgroundOperation::assertNoBgOpInProgForNs(ns()); invariant(_indexCatalog->numIndexesInProgress(opCtx) == 0); // 1) store index specs @@ -814,6 +815,7 @@ Status CollectionImpl::truncate(OperationContext* opCtx) { void CollectionImpl::cappedTruncateAfter(OperationContext* opCtx, RecordId end, bool inclusive) { dassert(opCtx->lockState()->isCollectionLockedForMode(ns(), MODE_X)); invariant(isCapped()); + BackgroundOperation::assertNoBgOpInProgForNs(ns()); invariant(_indexCatalog->numIndexesInProgress(opCtx) == 0); _recordStore->cappedTruncateAfter(opCtx, end, inclusive); diff --git a/src/mongo/db/catalog/collection_impl.h b/src/mongo/db/catalog/collection_impl.h index 73861c965de..1260150ca49 100644 --- a/src/mongo/db/catalog/collection_impl.h +++ b/src/mongo/db/catalog/collection_impl.h @@ -224,9 +224,6 @@ public: * removes all documents as fast as possible * indexes before and after will be the same * as will other characteristics - * - * The caller should hold a collection X lock and ensure there are no index builds in progress - * on the collection. */ Status truncate(OperationContext* opCtx) final; @@ -255,9 +252,6 @@ public: * collection. The collection cannot be completely emptied using this * function. An assertion will be thrown if that is attempted. * @param inclusive - Truncate 'end' as well iff true - * - * The caller should hold a collection X lock and ensure there are no index builds in progress - * on the collection. */ void cappedTruncateAfter(OperationContext* opCtx, RecordId end, bool inclusive) final; diff --git a/src/mongo/db/catalog/database.h b/src/mongo/db/catalog/database.h index 4e4245f4625..30648c41bd9 100644 --- a/src/mongo/db/catalog/database.h +++ b/src/mongo/db/catalog/database.h @@ -176,9 +176,6 @@ public: * * If we are applying a 'drop' oplog entry on a secondary, 'dropOpTime' will contain the optime * of the oplog entry. - * - * The caller should hold a DB X lock and ensure there are no index builds in progress on the - * collection. */ virtual Status dropCollection(OperationContext* const opCtx, const StringData fullns, diff --git a/src/mongo/db/catalog/database_holder.h b/src/mongo/db/catalog/database_holder.h index fdadd00bd0f..0ddefd433f5 100644 --- a/src/mongo/db/catalog/database_holder.h +++ b/src/mongo/db/catalog/database_holder.h @@ -81,8 +81,7 @@ public: * doesn't notify the replication subsystem or do any other consistency checks, so it should * not be used directly from user commands. * - * Must be called with the specified database locked in X mode. The caller must ensure no index - * builds are in progress on the database. + * Must be called with the specified database locked in X mode. */ virtual void dropDb(OperationContext* opCtx, Database* db) = 0; @@ -95,8 +94,6 @@ public: /** * Closes all opened databases. Must be called with the global lock acquired in X-mode. * Will uassert if any background jobs are running when this function is called. - * - * The caller must hold the global X lock and ensure there are no index builds in progress. */ virtual void closeAll(OperationContext* opCtx) = 0; diff --git a/src/mongo/db/catalog/database_holder_impl.cpp b/src/mongo/db/catalog/database_holder_impl.cpp index d333d8bfe0f..f518c6c0f13 100644 --- a/src/mongo/db/catalog/database_holder_impl.cpp +++ b/src/mongo/db/catalog/database_holder_impl.cpp @@ -170,12 +170,7 @@ void DatabaseHolderImpl::dropDb(OperationContext* opCtx, Database* db) { invariant(opCtx->lockState()->isDbLockedForMode(name, MODE_X)); - for (auto&& coll : *db) { - // It is the caller's responsibility to ensure that no index builds are active in the - // database. - invariant(!coll->getIndexCatalog()->haveAnyIndexesInProgress(), - str::stream() << "An index is building on collection '" << coll->ns() << "'."); - } + BackgroundOperation::assertNoBgOpInProgForDb(name); audit::logDropDatabase(opCtx->getClient(), name); @@ -237,13 +232,7 @@ void DatabaseHolderImpl::closeAll(OperationContext* opCtx) { std::set dbs; for (DBs::const_iterator i = _dbs.begin(); i != _dbs.end(); ++i) { - for (auto&& coll : *(i->second)) { - // It is the caller's responsibility to ensure that no index builds are active in the - // database. - invariant(!coll->getIndexCatalog()->haveAnyIndexesInProgress(), - str::stream() << "An index is building on collection '" << coll->ns() - << "'."); - } + BackgroundOperation::assertNoBgOpInProgForDb(i->first); dbs.insert(i->first); } diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index 8833c4fea54..e8db845ce50 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -515,10 +515,12 @@ Status DatabaseImpl::dropCollectionEvenIfSystem(OperationContext* opCtx, uassertNamespaceNotIndex(fullns.toString(), "dropCollection"); + BackgroundOperation::assertNoBgOpInProgForNs(fullns); + // Make sure no indexes builds are in progress. // Use massert() to be consistent with IndexCatalog::dropAllIndexes(). auto numIndexesInProgress = collection->getIndexCatalog()->numIndexesInProgress(opCtx); - massert(ErrorCodes::BackgroundOperationInProgressForNamespace, + massert(40461, str::stream() << "cannot drop collection " << fullns << " (" << uuidString << ") when " << numIndexesInProgress << " index builds in progress.", @@ -693,6 +695,8 @@ Status DatabaseImpl::renameCollection(OperationContext* opCtx, bool stayTemp) { audit::logRenameCollection(&cc(), fromNS, toNS); invariant(opCtx->lockState()->isDbLockedForMode(name(), MODE_X)); + BackgroundOperation::assertNoBgOpInProgForNs(fromNS); + BackgroundOperation::assertNoBgOpInProgForNs(toNS); const NamespaceString fromNSS(fromNS); const NamespaceString toNSS(toNS); @@ -709,20 +713,6 @@ Status DatabaseImpl::renameCollection(OperationContext* opCtx, if (!collToRename) { return Status(ErrorCodes::NamespaceNotFound, "collection not found to rename"); } - invariant(!collToRename->getIndexCatalog()->haveAnyIndexesInProgress(), - mongoutils::str::stream() - << "cannot perform operation: an index build is currently running for " - "collection " - << fromNSS); - - Collection* toColl = getCollection(opCtx, toNSS); - if (toColl) { - invariant(!toColl->getIndexCatalog()->haveAnyIndexesInProgress(), - mongoutils::str::stream() - << "cannot perform operation: an index build is currently running for " - "collection " - << toNSS); - } log() << "renameCollection: renaming collection " << collToRename->uuid()->toString() << " from " << fromNS << " to " << toNS; diff --git a/src/mongo/db/catalog/database_impl.h b/src/mongo/db/catalog/database_impl.h index 09b5441c0bf..31e04fb8476 100644 --- a/src/mongo/db/catalog/database_impl.h +++ b/src/mongo/db/catalog/database_impl.h @@ -83,9 +83,6 @@ public: * * If we are applying a 'drop' oplog entry on a secondary, 'dropOpTime' will contain the optime * of the oplog entry. - * - * The caller should hold a DB X lock and ensure there are no index builds in progress on the - * collection. */ Status dropCollection(OperationContext* opCtx, StringData fullns, @@ -125,9 +122,6 @@ public: * Renames the fully qualified namespace 'fromNS' to the fully qualified namespace 'toNS'. * Illegal to call unless both 'fromNS' and 'toNS' are within this database. Returns an error if * 'toNS' already exists or 'fromNS' does not exist. - * - * The caller should hold a DB X lock and ensure there are no index builds in progress on either - * the 'fromNS' or the 'toNS'. */ Status renameCollection(OperationContext* opCtx, StringData fromNS, diff --git a/src/mongo/db/catalog/database_test.cpp b/src/mongo/db/catalog/database_test.cpp index 70af753caa0..e1743a76488 100644 --- a/src/mongo/db/catalog/database_test.cpp +++ b/src/mongo/db/catalog/database_test.cpp @@ -324,9 +324,7 @@ void _testDropCollectionThrowsExceptionIfThereAreIndexesInProgress(OperationCont ASSERT_GREATER_THAN(indexCatalog->numIndexesInProgress(opCtx), 0); WriteUnitOfWork wuow(opCtx); - ASSERT_THROWS_CODE(db->dropCollection(opCtx, nss.ns()), - AssertionException, - ErrorCodes::BackgroundOperationInProgressForNamespace); + ASSERT_THROWS_CODE(db->dropCollection(opCtx, nss.ns()), AssertionException, 40461); }); } diff --git a/src/mongo/db/catalog/drop_collection.cpp b/src/mongo/db/catalog/drop_collection.cpp index 110016b1474..93b281930a1 100644 --- a/src/mongo/db/catalog/drop_collection.cpp +++ b/src/mongo/db/catalog/drop_collection.cpp @@ -39,7 +39,6 @@ #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/curop.h" #include "mongo/db/db_raii.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/server_options.h" #include "mongo/db/service_context.h" @@ -99,8 +98,6 @@ Status dropCollection(OperationContext* opCtx, int numIndexes = coll->getIndexCatalog()->numIndexesTotal(opCtx); BackgroundOperation::assertNoBgOpInProgForNs(collectionName.ns()); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - coll->uuid().get()); Status s = systemCollectionMode == DropCollectionSystemCollectionMode::kDisallowSystemCollectionDrops diff --git a/src/mongo/db/catalog/drop_database.cpp b/src/mongo/db/catalog/drop_database.cpp index 2f2b9f79d29..13b8193f36c 100644 --- a/src/mongo/db/catalog/drop_database.cpp +++ b/src/mongo/db/catalog/drop_database.cpp @@ -41,7 +41,6 @@ #include "mongo/db/client.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/curop.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/op_observer.h" #include "mongo/db/repl/repl_client_info.h" #include "mongo/db/repl/replication_coordinator.h" @@ -66,10 +65,6 @@ Status _finishDropDatabase(OperationContext* opCtx, const std::string& dbName, Database* db, std::size_t numCollections) { - invariant(opCtx->lockState()->isDbLockedForMode(dbName, MODE_X)); - BackgroundOperation::assertNoBgOpInProgForDb(dbName); - IndexBuildsCoordinator::get(opCtx)->assertNoBgOpInProgForDb(dbName); - // If DatabaseHolder::dropDb() fails, we should reset the drop-pending state on Database. auto dropPendingGuard = makeGuard([db, opCtx] { db->setDropPending(opCtx, false); }); @@ -180,10 +175,6 @@ Status dropDatabase(OperationContext* opCtx, const std::string& dbName) { invariant(!nss.isReplicated() || nss.coll().startsWith("tmp.mr")); } - BackgroundOperation::assertNoBgOpInProgForNs(nss.ns()); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - db->getCollection(opCtx, nss)->uuid().get()); - WriteUnitOfWork wunit(opCtx); // A primary processing this will assign a timestamp when the operation is written to // the oplog. As stated above, a secondary processing must only observe non-replicated diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp index 56143447308..eb8049c3090 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -40,7 +40,6 @@ #include "mongo/db/curop.h" #include "mongo/db/db_raii.h" #include "mongo/db/index/index_descriptor.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/op_observer.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/service_context.h" @@ -226,12 +225,9 @@ Status dropIndexes(OperationContext* opCtx, return Status(ErrorCodes::NamespaceNotFound, "ns not found"); } - BackgroundOperation::assertNoBgOpInProgForNs(nss); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - collection->uuid().get()); - WriteUnitOfWork wunit(opCtx); OldClientContext ctx(opCtx, nss.ns()); + BackgroundOperation::assertNoBgOpInProgForNs(nss); Status status = wrappedRun(opCtx, collection, cmdObj, result); if (!status.isOK()) { diff --git a/src/mongo/db/catalog/index_catalog.h b/src/mongo/db/catalog/index_catalog.h index 5ba372df577..5ee4aed313c 100644 --- a/src/mongo/db/catalog/index_catalog.h +++ b/src/mongo/db/catalog/index_catalog.h @@ -218,8 +218,6 @@ public: virtual bool haveAnyIndexes() const = 0; - virtual bool haveAnyIndexesInProgress() const = 0; - virtual int numIndexesTotal(OperationContext* const opCtx) const = 0; virtual int numIndexesReady(OperationContext* const opCtx) const = 0; @@ -303,9 +301,6 @@ public: * Use this method to notify the IndexCatalog that the spec for this index has changed. * * It is invalid to dereference 'oldDesc' after calling this method. - * - * The caller must hold the collection X lock and ensure no index builds are in progress - * on the collection. */ virtual const IndexDescriptor* refreshEntry(OperationContext* const opCtx, const IndexDescriptor* const oldDesc) = 0; @@ -377,12 +372,6 @@ public: stdx::function onDropFn) = 0; virtual void dropAllIndexes(OperationContext* opCtx, bool includingIdIndex) = 0; - /** - * Drops the index. - * - * The caller must hold the collection X lock and ensure no index builds are in progress on the - * collection. - */ virtual Status dropIndex(OperationContext* const opCtx, const IndexDescriptor* const desc) = 0; /** diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp index 902daebfa3c..7f7d98b17c5 100644 --- a/src/mongo/db/catalog/index_catalog_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_impl.cpp @@ -740,10 +740,7 @@ void IndexCatalogImpl::dropAllIndexes(OperationContext* opCtx, stdx::function onDropFn) { invariant(opCtx->lockState()->isCollectionLockedForMode(_collection->ns(), MODE_X)); - uassert(ErrorCodes::BackgroundOperationInProgressForNamespace, - mongoutils::str::stream() - << "cannot perform operation: an index build is currently running", - !haveAnyIndexesInProgress()); + BackgroundOperation::assertNoBgOpInProgForNs(_collection->ns().ns()); // make sure nothing in progress massert(17348, @@ -816,7 +813,8 @@ void IndexCatalogImpl::dropAllIndexes(OperationContext* opCtx, bool includingIdI Status IndexCatalogImpl::dropIndex(OperationContext* opCtx, const IndexDescriptor* desc) { invariant(opCtx->lockState()->isCollectionLockedForMode(_collection->ns(), MODE_X)); - invariant(!haveAnyIndexesInProgress()); + BackgroundOperation::assertNoBgOpInProgForNs(_collection->ns().ns()); + invariant(_buildingIndexes.size() == 0); IndexCatalogEntry* entry = _readyIndexes.find(desc); @@ -962,10 +960,6 @@ bool IndexCatalogImpl::haveAnyIndexes() const { return _readyIndexes.size() > 0 || _buildingIndexes.size() > 0; } -bool IndexCatalogImpl::haveAnyIndexesInProgress() const { - return _buildingIndexes.size() > 0; -} - int IndexCatalogImpl::numIndexesTotal(OperationContext* opCtx) const { int count = _readyIndexes.size() + _buildingIndexes.size() + _unfinishedIndexes.size(); dassert(_collection->getCatalogEntry()->getTotalIndexCount(opCtx) == count); @@ -1130,6 +1124,7 @@ std::vector> IndexCatalogImpl::getAllRe const IndexDescriptor* IndexCatalogImpl::refreshEntry(OperationContext* opCtx, const IndexDescriptor* oldDesc) { invariant(opCtx->lockState()->isCollectionLockedForMode(_collection->ns(), MODE_X)); + invariant(!BackgroundOperation::inProgForNs(_collection->ns())); invariant(_buildingIndexes.size() == 0); const std::string indexName = oldDesc->indexName(); diff --git a/src/mongo/db/catalog/index_catalog_impl.h b/src/mongo/db/catalog/index_catalog_impl.h index d95f557102f..933bab58b62 100644 --- a/src/mongo/db/catalog/index_catalog_impl.h +++ b/src/mongo/db/catalog/index_catalog_impl.h @@ -67,7 +67,6 @@ public: // ---- accessors ----- bool haveAnyIndexes() const override; - bool haveAnyIndexesInProgress() const override; int numIndexesTotal(OperationContext* opCtx) const override; int numIndexesReady(OperationContext* opCtx) const override; int numIndexesInProgress(OperationContext* opCtx) const { @@ -155,9 +154,6 @@ public: * Use this method to notify the IndexCatalog that the spec for this index has changed. * * It is invalid to dereference 'oldDesc' after calling this method. - * - * The caller must hold the collection X lock and ensure no index builds are in progress - * on the collection. */ const IndexDescriptor* refreshEntry(OperationContext* opCtx, const IndexDescriptor* oldDesc) override; @@ -205,12 +201,6 @@ public: stdx::function onDropFn) override; void dropAllIndexes(OperationContext* opCtx, bool includingIdIndex) override; - /** - * Drops the index. - * - * The caller must hold the collection X lock and ensure no index builds are in progress on the - * collection. - */ Status dropIndex(OperationContext* opCtx, const IndexDescriptor* desc) override; /** diff --git a/src/mongo/db/catalog/index_catalog_noop.h b/src/mongo/db/catalog/index_catalog_noop.h index 445b26f384c..b1730c1fd05 100644 --- a/src/mongo/db/catalog/index_catalog_noop.h +++ b/src/mongo/db/catalog/index_catalog_noop.h @@ -51,10 +51,6 @@ public: return false; } - bool haveAnyIndexesInProgress() const override { - return false; - } - int numIndexesTotal(OperationContext* const opCtx) const override { return 0; } diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index 89e99ac6cd4..fcf5d8065ce 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -45,7 +45,6 @@ #include "mongo/db/curop.h" #include "mongo/db/db_raii.h" #include "mongo/db/index/index_descriptor.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/jsobj.h" #include "mongo/db/namespace_string.h" #include "mongo/db/ops/insert.h" @@ -203,8 +202,6 @@ Status renameCollectionCommon(OperationContext* opCtx, } BackgroundOperation::assertNoBgOpInProgForNs(source.ns()); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - sourceColl->uuid().get()); auto targetDB = databaseHolder->openDb(opCtx, target.db()); @@ -356,10 +353,6 @@ Status renameCollectionCommon(OperationContext* opCtx, // No logOp necessary because the entire renameCollection command is one logOp. repl::UnreplicatedWritesBlock uwb(opCtx); - BackgroundOperation::assertNoBgOpInProgForNs(targetColl->ns().ns()); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - targetColl->uuid().get()); - status = targetDB->dropCollection(opCtx, targetColl->ns().ns(), renameOpTime); if (!status.isOK()) { return status; diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 217db0ed3d5..dd933d76470 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -52,7 +52,6 @@ #include "mongo/db/dbhelpers.h" #include "mongo/db/exec/working_set_common.h" #include "mongo/db/index/index_descriptor.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/matcher/extensions_callback_real.h" #include "mongo/db/op_observer.h" #include "mongo/db/ops/insert.h" @@ -177,20 +176,15 @@ void dropTempCollections(OperationContext* cleanupOpCtx, [cleanupOpCtx, &tempNamespace] { AutoGetDb autoDb(cleanupOpCtx, tempNamespace.db(), MODE_X); if (auto db = autoDb.getDb()) { - if (auto collection = db->getCollection(cleanupOpCtx, tempNamespace)) { - uassert(ErrorCodes::PrimarySteppedDown, - str::stream() << "no longer primary while dropping temporary " - "collection for mapReduce: " - << tempNamespace.ns(), - repl::ReplicationCoordinator::get(cleanupOpCtx) - ->canAcceptWritesFor(cleanupOpCtx, tempNamespace)); - BackgroundOperation::assertNoBgOpInProgForNs(tempNamespace.ns()); - IndexBuildsCoordinator::get(cleanupOpCtx) - ->assertNoIndexBuildInProgForCollection(collection->uuid().get()); - WriteUnitOfWork wunit(cleanupOpCtx); - uassertStatusOK(db->dropCollection(cleanupOpCtx, tempNamespace.ns())); - wunit.commit(); - } + WriteUnitOfWork wunit(cleanupOpCtx); + uassert(ErrorCodes::PrimarySteppedDown, + str::stream() << "no longer primary while dropping temporary " + "collection for mapReduce: " + << tempNamespace.ns(), + repl::ReplicationCoordinator::get(cleanupOpCtx) + ->canAcceptWritesFor(cleanupOpCtx, tempNamespace)); + uassertStatusOK(db->dropCollection(cleanupOpCtx, tempNamespace.ns())); + wunit.commit(); } }); // Always forget about temporary namespaces, so we don't cache lots of them @@ -202,14 +196,9 @@ void dropTempCollections(OperationContext* cleanupOpCtx, Lock::DBLock lk(cleanupOpCtx, incLong.db(), MODE_X); auto databaseHolder = DatabaseHolder::get(cleanupOpCtx); if (auto db = databaseHolder->getDb(cleanupOpCtx, incLong.ns())) { - if (auto collection = db->getCollection(cleanupOpCtx, incLong)) { - BackgroundOperation::assertNoBgOpInProgForNs(incLong.ns()); - IndexBuildsCoordinator::get(cleanupOpCtx) - ->assertNoIndexBuildInProgForCollection(collection->uuid().get()); - WriteUnitOfWork wunit(cleanupOpCtx); - uassertStatusOK(db->dropCollection(cleanupOpCtx, incLong.ns())); - wunit.commit(); - } + WriteUnitOfWork wunit(cleanupOpCtx); + uassertStatusOK(db->dropCollection(cleanupOpCtx, incLong.ns())); + wunit.commit(); } }); diff --git a/src/mongo/db/commands/rename_collection_cmd.cpp b/src/mongo/db/commands/rename_collection_cmd.cpp index 7107a370ce0..fe2a048b153 100644 --- a/src/mongo/db/commands/rename_collection_cmd.cpp +++ b/src/mongo/db/commands/rename_collection_cmd.cpp @@ -80,6 +80,14 @@ public: return CommandHelpers::parseNsFullyQualified(cmdObj); } + static void dropCollection(OperationContext* opCtx, Database* db, StringData collName) { + WriteUnitOfWork wunit(opCtx); + if (db->dropCollection(opCtx, collName).isOK()) { + // ignoring failure case + wunit.commit(); + } + } + virtual bool errmsgRun(OperationContext* opCtx, const string& dbname, const BSONObj& cmdObj, diff --git a/src/mongo/db/commands/test_commands.cpp b/src/mongo/db/commands/test_commands.cpp index 29a3be76ea2..99c3b94f1e7 100644 --- a/src/mongo/db/commands/test_commands.cpp +++ b/src/mongo/db/commands/test_commands.cpp @@ -41,7 +41,6 @@ #include "mongo/db/commands.h" #include "mongo/db/commands/test_commands_enabled.h" #include "mongo/db/db_raii.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/op_observer.h" #include "mongo/db/query/internal_plans.h" #include "mongo/db/service_context.h" @@ -170,10 +169,6 @@ public: } } - BackgroundOperation::assertNoBgOpInProgForNs(fullNs.ns()); - IndexBuildsCoordinator::get(opCtx)->assertNoIndexBuildInProgForCollection( - collection->uuid().get()); - collection->cappedTruncateAfter(opCtx, end, inc); return true; diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 5082c3d318f..b606277d287 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -67,6 +67,25 @@ namespace { constexpr auto kUniqueFieldName = "unique"_sd; constexpr auto kKeyFieldName = "key"_sd; +/** + * Returns the collection UUID for the given 'nss', or a NamespaceNotFound error. + * + * Momentarily takes the collection IS lock for 'nss' to access the collection UUID. + */ +StatusWith getCollectionUUID(OperationContext* opCtx, const NamespaceString& nss) { + try { + AutoGetCollection autoColl(opCtx, nss, MODE_IS); + auto collection = autoColl.getCollection(); + if (!collection) { + return {ErrorCodes::NamespaceNotFound, nss.ns()}; + } + return collection->uuid().get(); + } catch (const DBException& ex) { + invariant(ex.toStatus().code() == ErrorCodes::NamespaceNotFound); + return ex.toStatus(); + } +} + /** * Checks if unique index specification is compatible with sharding configuration. */ @@ -263,8 +282,6 @@ void IndexBuildsCoordinator::interruptAllIndexBuilds(const std::string& reason) // Wait for all the index builds to stop. for (auto& dbIt : _databaseIndexBuilds) { - // Take a shared ptr, rather than accessing the Tracker through the map's iterator, so that - // the object does not destruct while we are waiting, causing a use-after-free memory error. auto dbIndexBuildsSharedPtr = dbIt.second; dbIndexBuildsSharedPtr->waitUntilNoIndexBuildsRemain(lk); } @@ -285,8 +302,6 @@ void IndexBuildsCoordinator::abortCollectionIndexBuilds(const UUID& collectionUU collIndexBuildsIt->second->runOperationOnAllBuilds( lk, &_indexBuildsManager, abortIndexBuild, reason); - // Take a shared ptr, rather than accessing the Tracker through the map's iterator, so that the - // object does not destruct while we are waiting, causing a use-after-free memory error. auto collIndexBuildsSharedPtr = collIndexBuildsIt->second; collIndexBuildsSharedPtr->waitUntilNoIndexBuildsRemain(lk); } @@ -304,9 +319,6 @@ void IndexBuildsCoordinator::abortDatabaseIndexBuilds(StringData db, const std:: } dbIndexBuilds->runOperationOnAllBuilds(lk, &_indexBuildsManager, abortIndexBuild, reason); - - // '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); } @@ -369,15 +381,6 @@ bool IndexBuildsCoordinator::inProgForDb(StringData db) const { return _databaseIndexBuilds.find(db) != _databaseIndexBuilds.end(); } -void IndexBuildsCoordinator::assertNoIndexBuildInProgress() const { - stdx::unique_lock lk(_mutex); - uassert(ErrorCodes::BackgroundOperationInProgressForDatabase, - mongoutils::str::stream() << "cannot perform operation: there are currently " - << _allIndexBuilds.size() - << " index builds running.", - _allIndexBuilds.size() == 0); -} - void IndexBuildsCoordinator::assertNoIndexBuildInProgForCollection( const UUID& collectionUUID) const { uassert(ErrorCodes::BackgroundOperationInProgressForNamespace, @@ -395,17 +398,21 @@ void IndexBuildsCoordinator::assertNoBgOpInProgForDb(StringData db) const { !inProgForDb(db)); } -void IndexBuildsCoordinator::awaitNoIndexBuildInProgressForCollection( - const UUID& collectionUUID) const { +void IndexBuildsCoordinator::awaitNoBgOpInProgForNs(OperationContext* opCtx, StringData ns) const { + auto statusWithCollectionUUID = getCollectionUUID(opCtx, NamespaceString(ns)); + if (!statusWithCollectionUUID.isOK()) { + // The collection does not exist, so there are no index builds on it. + invariant(statusWithCollectionUUID.getStatus().code() == ErrorCodes::NamespaceNotFound); + return; + } + stdx::unique_lock lk(_mutex); - auto collIndexBuildsIt = _collectionIndexBuilds.find(collectionUUID); + auto collIndexBuildsIt = _collectionIndexBuilds.find(statusWithCollectionUUID.getValue()); if (collIndexBuildsIt == _collectionIndexBuilds.end()) { return; } - // Take a shared ptr, rather than accessing the Tracker through the map's iterator, so that the - // object does not destruct while we are waiting, causing a use-after-free memory error. auto collIndexBuildsSharedPtr = collIndexBuildsIt->second; collIndexBuildsSharedPtr->waitUntilNoIndexBuildsRemain(lk); } @@ -414,12 +421,10 @@ void IndexBuildsCoordinator::awaitNoBgOpInProgForDb(StringData db) const { stdx::unique_lock lk(_mutex); auto dbIndexBuildsIt = _databaseIndexBuilds.find(db); - if (dbIndexBuildsIt == _databaseIndexBuilds.end()) { + if (dbIndexBuildsIt != _databaseIndexBuilds.end()) { return; } - // Take a shared ptr, rather than accessing the Tracker through the map's iterator, so that the - // object does not destruct while we are waiting, causing a use-after-free memory error. auto dbIndexBuildsSharedPtr = dbIndexBuildsIt->second; dbIndexBuildsSharedPtr->waitUntilNoIndexBuildsRemain(lk); } diff --git a/src/mongo/db/index_builds_coordinator.h b/src/mongo/db/index_builds_coordinator.h index d49178017da..6ed20f5c3b8 100644 --- a/src/mongo/db/index_builds_coordinator.h +++ b/src/mongo/db/index_builds_coordinator.h @@ -262,11 +262,6 @@ public: */ bool inProgForDb(StringData db) const; - /** - * Uasserts if any index builds are in progress on any database. - */ - void assertNoIndexBuildInProgress() const; - /** * Uasserts if any index builds is in progress on the specified collection. */ @@ -279,8 +274,13 @@ public: /** * Waits for all index builds on a specified collection to finish. + * + * Momentarily takes the collection IS lock for 'ns', to fetch the collection UUID. */ - void awaitNoIndexBuildInProgressForCollection(const UUID& collectionUUID) const; + void awaitNoBgOpInProgForNs(OperationContext* opCtx, StringData ns) const; + void awaitNoBgOpInProgForNs(OperationContext* opCtx, const NamespaceString& ns) const { + awaitNoBgOpInProgForNs(opCtx, ns.ns()); + } /** * Waits for all index builds on a specified database to finish. diff --git a/src/mongo/db/repl/SConscript b/src/mongo/db/repl/SConscript index 3df073fdf83..b24fddfcfce 100644 --- a/src/mongo/db/repl/SConscript +++ b/src/mongo/db/repl/SConscript @@ -629,7 +629,6 @@ env.Library( ], LIBDEPS_PRIVATE=[ 'drop_pending_collection_reaper', - '$BUILD_DIR/mongo/db/index_builds_coordinator_interface', '$BUILD_DIR/mongo/idl/server_parameter', ], ) diff --git a/src/mongo/db/repl/apply_ops.cpp b/src/mongo/db/repl/apply_ops.cpp index e4ea07d203e..82c5717eb8a 100644 --- a/src/mongo/db/repl/apply_ops.cpp +++ b/src/mongo/db/repl/apply_ops.cpp @@ -46,7 +46,6 @@ #include "mongo/db/db_raii.h" #include "mongo/db/dbdirectclient.h" #include "mongo/db/index/index_descriptor.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/matcher/matcher.h" #include "mongo/db/op_observer.h" #include "mongo/db/operation_context.h" @@ -299,9 +298,9 @@ Status _applyPrepareTransaction(OperationContext* opCtx, // commits. for (const auto& opObj : info.getOperations()) { auto ns = opObj.getField("ns").checkAndGetStringData(); - auto uuid = uassertStatusOK(UUID::parse(opObj.getField("ui"))); - BackgroundOperation::awaitNoBgOpInProgForNs(ns); - IndexBuildsCoordinator::get(opCtx)->awaitNoIndexBuildInProgressForCollection(uuid); + if (BackgroundOperation::inProgForNs(ns)) { + BackgroundOperation::awaitNoBgOpInProgForNs(ns); + } } // Transaction operations are in its own batch, so we can modify their opCtx. diff --git a/src/mongo/db/repl/idempotency_test_fixture.cpp b/src/mongo/db/repl/idempotency_test_fixture.cpp index b47d798d3ee..6d12069a194 100644 --- a/src/mongo/db/repl/idempotency_test_fixture.cpp +++ b/src/mongo/db/repl/idempotency_test_fixture.cpp @@ -387,7 +387,7 @@ OplogEntry IdempotencyTest::update(IdType _id, const BSONObj& obj) { OplogEntry IdempotencyTest::buildIndex(const BSONObj& indexSpec, const BSONObj& options, - const UUID& uuid) { + UUID uuid) { BSONObjBuilder bob; bob.append("createIndexes", nss.coll()); bob.append("v", 2); @@ -397,9 +397,9 @@ OplogEntry IdempotencyTest::buildIndex(const BSONObj& indexSpec, return makeCommandOplogEntry(nextOpTime(), nss, bob.obj(), uuid); } -OplogEntry IdempotencyTest::dropIndex(const std::string& indexName, const UUID& uuid) { +OplogEntry IdempotencyTest::dropIndex(const std::string& indexName) { auto cmd = BSON("dropIndexes" << nss.coll() << "index" << indexName); - return makeCommandOplogEntry(nextOpTime(), nss, cmd, uuid); + return makeCommandOplogEntry(nextOpTime(), nss, cmd); } std::string IdempotencyTest::computeDataHash(Collection* collection) { @@ -431,19 +431,8 @@ std::string IdempotencyTest::computeDataHash(Collection* collection) { } CollectionState IdempotencyTest::validate() { - auto collUUID = [&]() -> OptionalCollectionUUID { - AutoGetCollectionForReadCommand autoColl(_opCtx.get(), nss); - if (auto collection = autoColl.getCollection()) { - return collection->uuid(); - } - return boost::none; - }(); - - if (collUUID) { - // Allow in-progress indexes to complete before validating collection contents. - IndexBuildsCoordinator::get(_opCtx.get()) - ->awaitNoIndexBuildInProgressForCollection(collUUID.get()); - } + // Allow in-progress indexes to complete before validating collection contents. + IndexBuildsCoordinator::get(_opCtx.get())->awaitNoBgOpInProgForNs(_opCtx.get(), nss); AutoGetCollectionForReadCommand autoColl(_opCtx.get(), nss); auto collection = autoColl.getCollection(); diff --git a/src/mongo/db/repl/idempotency_test_fixture.h b/src/mongo/db/repl/idempotency_test_fixture.h index d86278c9810..2823f31cb41 100644 --- a/src/mongo/db/repl/idempotency_test_fixture.h +++ b/src/mongo/db/repl/idempotency_test_fixture.h @@ -95,8 +95,8 @@ protected: OplogEntry insert(const BSONObj& obj); template OplogEntry update(IdType _id, const BSONObj& obj); - OplogEntry buildIndex(const BSONObj& indexSpec, const BSONObj& options, const UUID& uuid); - OplogEntry dropIndex(const std::string& indexName, const UUID& uuid); + OplogEntry buildIndex(const BSONObj& indexSpec, const BSONObj& options, UUID uuid); + OplogEntry dropIndex(const std::string& indexName); virtual Status resetState(); /** diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp index e45712ff716..35536687009 100644 --- a/src/mongo/db/repl/oplog.cpp +++ b/src/mongo/db/repl/oplog.cpp @@ -1981,7 +1981,6 @@ Status applyCommand_inlock(OperationContext* opCtx, Lock::TempRelease release(opCtx->lockState()); BackgroundOperation::awaitNoBgOpInProgForDb(nss.db()); - IndexBuildsCoordinator::get(opCtx)->awaitNoBgOpInProgForDb(nss.db()); opCtx->recoveryUnit()->abandonSnapshot(); opCtx->checkForInterrupt(); break; @@ -1993,16 +1992,8 @@ Status applyCommand_inlock(OperationContext* opCtx, invariant(cmd); // TODO: This parse could be expensive and not worth it. - auto ns = - cmd->parse(opCtx, OpMsgRequest::fromDBAndBody(nss.db(), o))->ns().toString(); - auto swUUID = UUID::parse(fieldUI); - if (!swUUID.isOK()) { - error() << "Failed command " << redact(o) << " on " << ns << " with status " - << swUUID.getStatus() << "during oplog application. Expected a UUID."; - } - BackgroundOperation::awaitNoBgOpInProgForNs(ns); - IndexBuildsCoordinator::get(opCtx)->awaitNoIndexBuildInProgressForCollection( - swUUID.getValue()); + BackgroundOperation::awaitNoBgOpInProgForNs( + cmd->parse(opCtx, OpMsgRequest::fromDBAndBody(nss.db(), o))->ns().toString()); opCtx->recoveryUnit()->abandonSnapshot(); opCtx->checkForInterrupt(); diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp index def645df987..f8092ab3896 100644 --- a/src/mongo/db/repl/rollback_impl.cpp +++ b/src/mongo/db/repl/rollback_impl.cpp @@ -43,7 +43,6 @@ #include "mongo/db/concurrency/d_concurrency.h" #include "mongo/db/concurrency/replication_state_transition_lock_guard.h" #include "mongo/db/db_raii.h" -#include "mongo/db/index_builds_coordinator.h" #include "mongo/db/kill_sessions_local.h" #include "mongo/db/logical_time_validator.h" #include "mongo/db/operation_context.h" @@ -376,13 +375,10 @@ Status RollbackImpl::_awaitBgIndexCompletion(OperationContext* opCtx) { log() << "Waiting for all background operations to complete before starting rollback"; for (auto db : dbNames) { auto numInProg = BackgroundOperation::numInProgForDb(db); - auto numInProgInCoordinator = IndexBuildsCoordinator::get(opCtx)->numInProgForDb(db); - if (numInProg > 0 || numInProgInCoordinator > 0) { - LOG(1) << "Waiting for " - << (numInProg > numInProgInCoordinator ? numInProg : numInProgInCoordinator) + if (numInProg > 0) { + LOG(1) << "Waiting for " << numInProg << " background operations to complete on database '" << db << "'"; BackgroundOperation::awaitNoBgOpInProgForDb(db); - IndexBuildsCoordinator::get(opCtx)->awaitNoBgOpInProgForDb(db); } // Check for shutdown again. diff --git a/src/mongo/db/repl/sync_tail_test.cpp b/src/mongo/db/repl/sync_tail_test.cpp index 368080f8b1e..f080f656c4d 100644 --- a/src/mongo/db/repl/sync_tail_test.cpp +++ b/src/mongo/db/repl/sync_tail_test.cpp @@ -1101,7 +1101,7 @@ TEST_F(IdempotencyTest, Geo2dsphereIndexFailedOnIndexing) { ASSERT_OK(runOpInitialSync(createCollection(kUuid))); auto indexOp = buildIndex(fromjson("{loc: '2dsphere'}"), BSON("2dsphereIndexVersion" << 3), kUuid); - auto dropIndexOp = dropIndex("loc_index", kUuid); + auto dropIndexOp = dropIndex("loc_index"); auto insertOp = insert(fromjson("{_id: 1, loc: 'hi'}")); auto ops = {indexOp, dropIndexOp, insertOp}; @@ -1174,7 +1174,7 @@ TEST_F(IdempotencyTest, IndexWithDifferentOptions) { auto indexOp1 = buildIndex(fromjson("{x: 'text'}"), fromjson("{default_language: 'spanish'}"), kUuid); - auto dropIndexOp = dropIndex("x_index", kUuid); + auto dropIndexOp = dropIndex("x_index"); auto indexOp2 = buildIndex(fromjson("{x: 'text'}"), fromjson("{default_language: 'english'}"), kUuid); @@ -1209,7 +1209,7 @@ TEST_F(IdempotencyTest, InsertDocumentWithNonStringLanguageFieldWhenTextIndexExi ASSERT_OK(runOpInitialSync(createCollection(kUuid))); auto indexOp = buildIndex(fromjson("{x: 'text'}"), BSONObj(), kUuid); - auto dropIndexOp = dropIndex("x_index", kUuid); + auto dropIndexOp = dropIndex("x_index"); auto insertOp = insert(fromjson("{_id: 1, x: 'words to index', language: 1}")); auto ops = {indexOp, dropIndexOp, insertOp}; @@ -1243,7 +1243,7 @@ TEST_F(IdempotencyTest, InsertDocumentWithNonStringLanguageOverrideFieldWhenText ASSERT_OK(runOpInitialSync(createCollection(kUuid))); auto indexOp = buildIndex(fromjson("{x: 'text'}"), fromjson("{language_override: 'y'}"), kUuid); - auto dropIndexOp = dropIndex("x_index", kUuid); + auto dropIndexOp = dropIndex("x_index"); auto insertOp = insert(fromjson("{_id: 1, x: 'words to index', y: 1}")); auto ops = {indexOp, dropIndexOp, insertOp}; @@ -1411,8 +1411,8 @@ TEST_F(IdempotencyTest, CollModNamespaceNotFound) { auto indexChange = fromjson("{keyPattern: {createdAt:1}, expireAfterSeconds:4000}}"); auto collModCmd = BSON("collMod" << nss.coll() << "index" << indexChange); - auto collModOp = makeCommandOplogEntry(nextOpTime(), nss, collModCmd, kUuid); - auto dropCollOp = makeCommandOplogEntry(nextOpTime(), nss, BSON("drop" << nss.coll()), kUuid); + auto collModOp = makeCommandOplogEntry(nextOpTime(), nss, collModCmd); + auto dropCollOp = makeCommandOplogEntry(nextOpTime(), nss, BSON("drop" << nss.coll())); auto ops = {collModOp, dropCollOp}; testOpsAreIdempotent(ops); @@ -1428,8 +1428,8 @@ TEST_F(IdempotencyTest, CollModIndexNotFound) { auto indexChange = fromjson("{keyPattern: {createdAt:1}, expireAfterSeconds:4000}}"); auto collModCmd = BSON("collMod" << nss.coll() << "index" << indexChange); - auto collModOp = makeCommandOplogEntry(nextOpTime(), nss, collModCmd, kUuid); - auto dropIndexOp = dropIndex("createdAt_index", kUuid); + auto collModOp = makeCommandOplogEntry(nextOpTime(), nss, collModCmd); + auto dropIndexOp = dropIndex("createdAt_index"); auto ops = {collModOp, dropIndexOp}; testOpsAreIdempotent(ops); -- cgit v1.2.1