diff options
author | Dianna <dianna.hohensee@10gen.com> | 2019-05-01 10:50:06 -0400 |
---|---|---|
committer | Dianna <dianna.hohensee@10gen.com> | 2019-05-13 17:08:05 -0400 |
commit | 3dbaffb7768951c185b48aa0bfd5a69dd99d4ec3 (patch) | |
tree | 66dadc92fd5a750b67911fc9d42b78af03c14587 | |
parent | dde091c07989ffaefc57705859abf6517beeeace (diff) | |
download | mongo-3dbaffb7768951c185b48aa0bfd5a69dd99d4ec3.tar.gz |
SERVER-40926 SERVER-40927 createIndexes cmd requests for indexes that are already being built wait for them to complete rather than return OK immediately
-rw-r--r-- | jstests/noPassthroughWithMongod/create_indexes_waits_for_already_in_progress.js | 135 | ||||
-rw-r--r-- | src/mongo/db/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/background.cpp | 29 | ||||
-rw-r--r-- | src/mongo/db/background.h | 11 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_catalog.h | 21 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_catalog_impl.cpp | 38 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_catalog_impl.h | 16 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_catalog_noop.h | 8 | ||||
-rw-r--r-- | src/mongo/db/catalog/multi_index_block.cpp | 28 | ||||
-rw-r--r-- | src/mongo/db/commands/create_indexes.cpp | 50 | ||||
-rw-r--r-- | src/mongo/db/index_builds_coordinator.cpp | 3 |
11 files changed, 302 insertions, 38 deletions
diff --git a/jstests/noPassthroughWithMongod/create_indexes_waits_for_already_in_progress.js b/jstests/noPassthroughWithMongod/create_indexes_waits_for_already_in_progress.js new file mode 100644 index 00000000000..8b82392ac5a --- /dev/null +++ b/jstests/noPassthroughWithMongod/create_indexes_waits_for_already_in_progress.js @@ -0,0 +1,135 @@ +/** + * Tests that a second duplicate createIndexes cmd request will wait for the first createIndexes cmd + * request to finish before proceeding to either: return OK; or try to build the index again. + * + * Sets up paused index builds via failpoints and a parallel shell. + * + * First tests that the second request returns OK after finding the index ready after waiting; + * then tests that the second request builds the index after waiting and finding the index does + * not exist. + * + * @tags: [ + * # Uses failpoints that the mongos does not have. + * assumes_against_mongod_not_mongos, + * # Sets a failpoint on one mongod, so switching primaries would break the test. + * does_not_support_stepdowns, + * # A write takes a global exclusive lock on the mobile engine, so two concurrent writers + * # (index builds) are impossible. + * requires_db_locking, + * ] + */ + +(function() { + "use strict"; + + load("jstests/libs/check_log.js"); + load("jstests/libs/parallel_shell_helpers.js"); + load('jstests/libs/test_background_ops.js'); + + const dbName = "test"; + const collName = "create_indexes_waits_for_already_in_progress"; + const testDB = db.getSiblingDB(dbName); + const testColl = testDB.getCollection(collName); + const indexSpecB = {key: {b: 1}, name: "the_b_1_index"}; + const indexSpecC = {key: {c: 1}, name: "the_c_1_index"}; + + testColl.drop(); + assert.commandWorked(testDB.adminCommand({clearLog: 'global'})); + + // TODO (SERVER-40952): currently createIndexes will hold an X lock for the duration of the + // build if the collection is not created beforehand. This test needs that not to happen, so we + // can pause a build and a subsequently issued request can get an IX lock. + assert.commandWorked(testDB.runCommand({create: collName})); + + function runSuccessfulIndexBuild(dbName, collName, indexSpec, requestNumber) { + jsTest.log("Index build request " + requestNumber + " starting..."); + const res = + db.getSiblingDB(dbName).runCommand({createIndexes: collName, indexes: [indexSpec]}); + jsTest.log("Index build request " + requestNumber + ", expected to succeed, result: " + + tojson(res)); + assert.commandWorked(res); + } + + assert.commandWorked(testDB.adminCommand( + {configureFailPoint: 'hangAfterSettingUpIndexBuild', mode: 'alwaysOn'})); + let joinFirstIndexBuild; + let joinSecondIndexBuild; + try { + jsTest.log("Starting a parallel shell to run first index build request..."); + joinFirstIndexBuild = startParallelShell( + funWithArgs(runSuccessfulIndexBuild, dbName, collName, indexSpecB, 1), + db.getMongo().port); + + jsTest.log("Waiting for first index build to get started..."); + checkLog.contains(db.getMongo(), + "Hanging index build due to failpoint 'hangAfterSettingUpIndexBuild'"); + + jsTest.log("Starting a parallel shell to run second index build request..."); + joinSecondIndexBuild = startParallelShell( + funWithArgs(runSuccessfulIndexBuild, dbName, collName, indexSpecB, 2), + db.getMongo().port); + + jsTest.log("Waiting for second index build request to wait behind the first..."); + checkLog.contains(db.getMongo(), + "but found that at least one of the indexes is already being built"); + } finally { + assert.commandWorked( + testDB.adminCommand({configureFailPoint: 'hangAfterSettingUpIndexBuild', mode: 'off'})); + } + + // The second request stalled behind the first, so now all we need to do is check that they both + // complete successfully. + joinFirstIndexBuild(); + joinSecondIndexBuild(); + + // Make sure the parallel shells sucessfully built the index. We should have the _id index and + // the 'the_b_1_index' index just built in the parallel shells. + assert.eq(testColl.getIndexes().length, 2); + + // Lastly, if the first request fails transiently, then the second should restart the index + // build. + assert.commandWorked(testDB.adminCommand({clearLog: 'global'})); + + function runFailedIndexBuild(dbName, collName, indexSpec, requestNumber) { + const res = + db.getSiblingDB(dbName).runCommand({createIndexes: collName, indexes: [indexSpec]}); + jsTest.log("Index build request " + requestNumber + ", expected to fail, result: " + + tojson(res)); + assert.commandFailedWithCode(res, ErrorCodes.InternalError); + } + + assert.commandWorked( + testDB.adminCommand({configureFailPoint: 'hangAndThenFailIndexBuild', mode: 'alwaysOn'})); + let joinFailedIndexBuild; + let joinSuccessfulIndexBuild; + try { + jsTest.log("Starting a parallel shell to run third index build request..."); + joinFailedIndexBuild = startParallelShell( + funWithArgs(runFailedIndexBuild, dbName, collName, indexSpecC, 3), db.getMongo().port); + + jsTest.log("Waiting for third index build to get started..."); + checkLog.contains(db.getMongo(), + "Hanging index build due to failpoint 'hangAndThenFailIndexBuild'"); + + jsTest.log("Starting a parallel shell to run fourth index build request..."); + joinSuccessfulIndexBuild = startParallelShell( + funWithArgs(runSuccessfulIndexBuild, dbName, collName, indexSpecC, 4), + db.getMongo().port); + + jsTest.log("Waiting for fourth index build request to wait behind the third..."); + checkLog.contains(db.getMongo(), + "but found that at least one of the indexes is already being built"); + } finally { + assert.commandWorked( + testDB.adminCommand({configureFailPoint: 'hangAndThenFailIndexBuild', mode: 'off'})); + } + + // The second request stalled behind the first, so now all we need to do is check that they both + // complete as expected: the first should fail; the second should succeed. + joinFailedIndexBuild(); + joinSuccessfulIndexBuild(); + + // Make sure the parallel shells sucessfully built the index. We should now have the _id index, + // the 'the_b_1_index' index and the 'the_c_1_index' just built in the parallel shells. + assert.eq(testColl.getIndexes().length, 3); +})(); diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 3f987df8840..0859eb526a9 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -927,6 +927,7 @@ env.Library( ], LIBDEPS=[ "$BUILD_DIR/mongo/base", + '$BUILD_DIR/mongo/db/service_context', ], ) diff --git a/src/mongo/db/background.cpp b/src/mongo/db/background.cpp index 78d10eda93e..0f7b9cd7eea 100644 --- a/src/mongo/db/background.cpp +++ b/src/mongo/db/background.cpp @@ -34,6 +34,7 @@ #include <iostream> #include <string> +#include "mongo/db/operation_context.h" #include "mongo/stdx/condition_variable.h" #include "mongo/stdx/mutex.h" #include "mongo/stdx/thread.h" @@ -44,8 +45,6 @@ namespace mongo { -using std::shared_ptr; - namespace { class BgInfo { @@ -53,7 +52,7 @@ class BgInfo { BgInfo& operator=(const BgInfo&) = delete; public: - BgInfo() : _opsInProgCount(0) {} + BgInfo() : _opsInProgCount(0), _opRemovalsCount(0) {} void recordBegin(); int recordEnd(); @@ -63,9 +62,13 @@ public: return _opsInProgCount; } + void waitForAnOpRemoval(stdx::unique_lock<stdx::mutex>& lk, OperationContext* opCtx); + private: int _opsInProgCount; stdx::condition_variable _noOpsInProg; + int _opRemovalsCount; + stdx::condition_variable _waitForOpRemoval; }; typedef StringMap<std::shared_ptr<BgInfo>> BgInfoMap; @@ -83,6 +86,8 @@ void BgInfo::recordBegin() { int BgInfo::recordEnd() { dassert(_opsInProgCount > 0); --_opsInProgCount; + ++_opRemovalsCount; + _waitForOpRemoval.notify_all(); if (0 == _opsInProgCount) { _noOpsInProg.notify_all(); } @@ -94,6 +99,14 @@ void BgInfo::awaitNoBgOps(stdx::unique_lock<stdx::mutex>& lk) { _noOpsInProg.wait(lk); } +void BgInfo::waitForAnOpRemoval(stdx::unique_lock<stdx::mutex>& lk, OperationContext* opCtx) { + int startOpRemovalsCount = _opRemovalsCount; + + // Wait for an index build to finish. + opCtx->waitForConditionOrInterrupt( + _waitForOpRemoval, lk, [&] { return startOpRemovalsCount != _opRemovalsCount; }); +} + void recordBeginAndInsert(BgInfoMap& bgiMap, StringData key) { std::shared_ptr<BgInfo>& bgInfo = bgiMap[key]; if (!bgInfo) @@ -118,6 +131,16 @@ void awaitNoBgOps(stdx::unique_lock<stdx::mutex>& lk, BgInfoMap* bgiMap, StringD } // namespace +void BackgroundOperation::waitUntilAnIndexBuildFinishes(OperationContext* opCtx, StringData ns) { + stdx::unique_lock<stdx::mutex> lk(m); + std::shared_ptr<BgInfo> bgInfo = mapFindWithDefault(nsInProg, ns, std::shared_ptr<BgInfo>()); + if (!bgInfo) { + // There are no index builds in progress on the collection, so no need to wait. + return; + } + bgInfo->waitForAnOpRemoval(lk, opCtx); +} + bool BackgroundOperation::inProgForDb(StringData db) { stdx::lock_guard<stdx::mutex> lk(m); return dbsInProg.find(db) != dbsInProg.end(); diff --git a/src/mongo/db/background.h b/src/mongo/db/background.h index 49bde198a44..d64430d93b0 100644 --- a/src/mongo/db/background.h +++ b/src/mongo/db/background.h @@ -44,6 +44,8 @@ namespace mongo { +class OperationContext; + /* these are administrative operations / jobs for a namespace running in the background, and that if in progress, you aren't allowed to do other NamespaceDetails major manipulations @@ -78,6 +80,15 @@ public: awaitNoBgOpInProgForNs(ns.ns()); } + /** + * Waits until an index build on collection 'ns' finishes. If there are no index builds in + * progress, returns immediately. + * + * Note: a collection lock should not be held when calling this, as that would block index + * builds from finishing and this function ever returning. + */ + static void waitUntilAnIndexBuildFinishes(OperationContext* opCtx, StringData ns); + /* check for in progress before instantiating */ BackgroundOperation(StringData ns); diff --git a/src/mongo/db/catalog/index_catalog.h b/src/mongo/db/catalog/index_catalog.h index 304ef4a09f8..9cc22948090 100644 --- a/src/mongo/db/catalog/index_catalog.h +++ b/src/mongo/db/catalog/index_catalog.h @@ -356,7 +356,8 @@ public: * fields. Lastly, checks whether the spec conflicts with ready and in-progress indexes. * * Returns an error Status or the cleaned up version of the non-conflicting spec. Returns - * IndexAlreadyExists if the index either already exists or is already being built. + * IndexAlreadyExists if the index already exists; IndexBuildAlreadyInProgress if the index is + * already being built. */ virtual StatusWith<BSONObj> prepareSpecForCreate(OperationContext* const opCtx, const BSONObj& original) const = 0; @@ -364,15 +365,21 @@ public: /** * Returns a copy of 'indexSpecsToBuild' that does not contain index specifications that already * exist or are already being built. If this is not done, an index build using - * 'indexSpecsToBuild' may fail with error code IndexAlreadyExists. If {buildIndexes:false} is - * set in the replica set config, also filters non-_id index specs out of the results. + * 'indexSpecsToBuild' may fail with an IndexAlreadyExists or IndexBuildAlreadyInProgress error. + * If {buildIndexes:false} is set in the replica set config, also filters non-_id index specs + * out of the results. * - * Additionally verifies the specs are valid and corrects any legacy fields. Throws on any spec - * validation errors or conflicts other than IndexAlreadyExists, which indicates that the index - * spec either already exists or is already being built and is what this function filters out. + * Additionally verifies the specs are valid. Throws on any spec validation errors or conflicts + * other than IndexAlreadyExists, which indicates that the index spec already exists is what + * this function filters out. + * + * 'removeIndexBuildsToo' controls whether in-progress index builds are also filtered out. If + * they are not, then IndexBuildAlreadyInProgress errors can be thrown. */ virtual std::vector<BSONObj> removeExistingIndexes( - OperationContext* const opCtx, const std::vector<BSONObj>& indexSpecsToBuild) const = 0; + OperationContext* const opCtx, + const std::vector<BSONObj>& indexSpecsToBuild, + const bool removeIndexBuildsToo) const = 0; /** * Filters out ready and in-progress indexes that already exist and returns the remaining diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp index 24287950d95..8d82670b009 100644 --- a/src/mongo/db/catalog/index_catalog_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_impl.cpp @@ -392,11 +392,27 @@ StatusWith<BSONObj> IndexCatalogImpl::prepareSpecForCreate(OperationContext* opC return status; } - status = _doesSpecConflictWithExisting(opCtx, swValidatedAndFixed.getValue()); + // First check against only the ready indexes for conflicts. + status = _doesSpecConflictWithExisting(opCtx, swValidatedAndFixed.getValue(), false); if (!status.isOK()) { return status; } + // Now we will check against all indexes, in-progress included. + // + // The index catalog cannot currently iterate over only in-progress indexes. So by previously + // checking against only ready indexes without error, we know that any errors encountered + // checking against all indexes occurred due to an in-progress index. + status = _doesSpecConflictWithExisting(opCtx, swValidatedAndFixed.getValue(), true); + if (!status.isOK()) { + if (ErrorCodes::IndexAlreadyExists == status.code()) { + // Callers need to be able to distinguish conflicts against ready indexes versus + // in-progress indexes. + return {ErrorCodes::IndexBuildAlreadyInProgress, status.reason()}; + } + return status; + } + return swValidatedAndFixed.getValue(); } @@ -413,7 +429,8 @@ std::vector<BSONObj> IndexCatalogImpl::removeExistingIndexesNoChecks( // _doesSpecConflictWithExisting currently does more work than we require here: we are only // interested in the index already exists error. - if (ErrorCodes::IndexAlreadyExists == _doesSpecConflictWithExisting(opCtx, spec)) { + if (ErrorCodes::IndexAlreadyExists == + _doesSpecConflictWithExisting(opCtx, spec, true /*includeUnfinishedIndexes*/)) { continue; } @@ -423,11 +440,14 @@ std::vector<BSONObj> IndexCatalogImpl::removeExistingIndexesNoChecks( } std::vector<BSONObj> IndexCatalogImpl::removeExistingIndexes( - OperationContext* const opCtx, const std::vector<BSONObj>& indexSpecsToBuild) const { + OperationContext* const opCtx, + const std::vector<BSONObj>& indexSpecsToBuild, + const bool removeIndexBuildsToo) const { std::vector<BSONObj> result; for (const auto& spec : indexSpecsToBuild) { auto prepareResult = prepareSpecForCreate(opCtx, spec); - if (prepareResult == ErrorCodes::IndexAlreadyExists) { + if (prepareResult == ErrorCodes::IndexAlreadyExists || + (removeIndexBuildsToo && prepareResult == ErrorCodes::IndexBuildAlreadyInProgress)) { continue; } uassertStatusOK(prepareResult); @@ -756,7 +776,8 @@ Status IndexCatalogImpl::_isSpecOk(OperationContext* opCtx, const BSONObj& spec) } Status IndexCatalogImpl::_doesSpecConflictWithExisting(OperationContext* opCtx, - const BSONObj& spec) const { + const BSONObj& spec, + const bool includeUnfinishedIndexes) const { const char* name = spec.getStringField("name"); invariant(name[0]); @@ -764,8 +785,7 @@ Status IndexCatalogImpl::_doesSpecConflictWithExisting(OperationContext* opCtx, const BSONObj collation = spec.getObjectField("collation"); { - const IndexDescriptor* desc = - findIndexByName(opCtx, name, true /*includeUnfinishedIndexes*/); + const IndexDescriptor* desc = findIndexByName(opCtx, name, includeUnfinishedIndexes); if (desc) { // index already exists with same name @@ -809,8 +829,8 @@ Status IndexCatalogImpl::_doesSpecConflictWithExisting(OperationContext* opCtx, } { - const IndexDescriptor* desc = findIndexByKeyPatternAndCollationSpec( - opCtx, key, collation, true /*includeUnfinishedIndexes*/); + const IndexDescriptor* desc = + findIndexByKeyPatternAndCollationSpec(opCtx, key, collation, includeUnfinishedIndexes); if (desc) { LOG(2) << "Index already exists with a different name: " << name << " pattern: " << key << " collation: " << collation; diff --git a/src/mongo/db/catalog/index_catalog_impl.h b/src/mongo/db/catalog/index_catalog_impl.h index 28a1233c58d..0b4ad27d872 100644 --- a/src/mongo/db/catalog/index_catalog_impl.h +++ b/src/mongo/db/catalog/index_catalog_impl.h @@ -191,9 +191,9 @@ public: StatusWith<BSONObj> prepareSpecForCreate(OperationContext* opCtx, const BSONObj& original) const override; - std::vector<BSONObj> removeExistingIndexes( - OperationContext* const opCtx, - const std::vector<BSONObj>& indexSpecsToBuild) const override; + std::vector<BSONObj> removeExistingIndexes(OperationContext* const opCtx, + const std::vector<BSONObj>& indexSpecsToBuild, + const bool removeIndexBuildsToo) const override; std::vector<BSONObj> removeExistingIndexesNoChecks( OperationContext* const opCtx, @@ -446,9 +446,15 @@ private: /** * Checks whether there are any spec conflicts with existing ready indexes or in-progress index * builds. Also checks whether any limits set on this server would be exceeded by building the - * index. + * index. 'includeUnfinishedIndexes' dictates whether in-progress index builds are checked for + * conflicts, along with ready indexes. + * + * Returns IndexAlreadyExists for both ready and in-progress index builds. Can also return other + * errors. */ - Status _doesSpecConflictWithExisting(OperationContext* opCtx, const BSONObj& spec) const; + Status _doesSpecConflictWithExisting(OperationContext* opCtx, + const BSONObj& spec, + const bool includeUnfinishedIndexes) const; /** * Returns true if the replica set member's config has {buildIndexes:false} set, which means diff --git a/src/mongo/db/catalog/index_catalog_noop.h b/src/mongo/db/catalog/index_catalog_noop.h index a1cb16bd9f4..9f9bb23c27b 100644 --- a/src/mongo/db/catalog/index_catalog_noop.h +++ b/src/mongo/db/catalog/index_catalog_noop.h @@ -147,10 +147,10 @@ public: return original; } - std::vector<BSONObj> removeExistingIndexes( - OperationContext* const opCtx, - const std::vector<BSONObj>& indexSpecsToBuild) const override { - return {}; + std::vector<BSONObj> removeExistingIndexes(OperationContext* const opCtx, + const std::vector<BSONObj>& indexSpecsToBuild, + const bool removeIndexBuildsToo) const override { + return indexSpecsToBuild; } std::vector<BSONObj> removeExistingIndexesNoChecks( diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp index 66b5c914a36..e2bf3d8fb25 100644 --- a/src/mongo/db/catalog/multi_index_block.cpp +++ b/src/mongo/db/catalog/multi_index_block.cpp @@ -61,10 +61,12 @@ namespace mongo { +MONGO_FAIL_POINT_DEFINE(hangAfterSettingUpIndexBuild); MONGO_FAIL_POINT_DEFINE(hangAfterStartingIndexBuild); MONGO_FAIL_POINT_DEFINE(hangAfterStartingIndexBuildUnlocked); MONGO_FAIL_POINT_DEFINE(hangBeforeIndexBuildOf); MONGO_FAIL_POINT_DEFINE(hangAfterIndexBuildOf); +MONGO_FAIL_POINT_DEFINE(hangAndThenFailIndexBuild); MONGO_FAIL_POINT_DEFINE(leaveIndexBuildUnfinishedForShutdown); MultiIndexBlock::~MultiIndexBlock() { @@ -282,8 +284,18 @@ StatusWith<std::vector<BSONObj>> MultiIndexBlock::init(OperationContext* opCtx, StatusWith<BSONObj> statusWithInfo = collection->getIndexCatalog()->prepareSpecForCreate(opCtx, info); Status status = statusWithInfo.getStatus(); - if (!status.isOK()) + if (!status.isOK()) { + // If we were given two identical indexes to build, we will run into an error trying to + // set up the same index a second time in this for-loop. This is the only way to + // encounter this error because callers filter out ready/in-progress indexes and start + // the build while holding a lock throughout. + if (status == ErrorCodes::IndexBuildAlreadyInProgress) { + invariant(indexSpecs.size() > 1); + return {ErrorCodes::OperationFailed, + "Cannot build two identical indexes. Try again without duplicate indexes."}; + } return status; + } info = statusWithInfo.getValue(); indexInfoObjs.push_back(info); @@ -387,6 +399,20 @@ Status MultiIndexBlock::insertAllDocumentsInCollection(OperationContext* opCtx, progress.set(CurOp::get(opCtx)->setProgress_inlock(curopMessage, numRecords)); } + if (MONGO_FAIL_POINT(hangAfterSettingUpIndexBuild)) { + // Hang the build after the BackgroundOperation and curOP info is set up. + log() << "Hanging index build due to failpoint 'hangAfterSettingUpIndexBuild'"; + MONGO_FAIL_POINT_PAUSE_WHILE_SET(hangAfterSettingUpIndexBuild); + } + + if (MONGO_FAIL_POINT(hangAndThenFailIndexBuild)) { + // Hang the build after the BackgroundOperation and curOP info is set up. + log() << "Hanging index build due to failpoint 'hangAndThenFailIndexBuild'"; + MONGO_FAIL_POINT_PAUSE_WHILE_SET(hangAndThenFailIndexBuild); + return {ErrorCodes::InternalError, + "Failed index build because of failpoint 'hangAndThenFailIndexBuild'"}; + } + Timer t; unsigned long long n = 0; diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index be34da6bf71..58d1125f0e4 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -36,6 +36,7 @@ #include "mongo/base/string_data.h" #include "mongo/db/auth/authorization_session.h" +#include "mongo/db/background.h" #include "mongo/db/catalog/collection.h" #include "mongo/db/catalog/database.h" #include "mongo/db/catalog/database_holder.h" @@ -213,7 +214,9 @@ std::vector<BSONObj> resolveDefaultsAndRemoveExistingIndexes(OperationContext* o uassertStatusOK(swDefaults.getStatus()); auto indexCatalog = collection->getIndexCatalog(); - return indexCatalog->removeExistingIndexes(opCtx, swDefaults.getValue()); + + return indexCatalog->removeExistingIndexes( + opCtx, swDefaults.getValue(), false /*removeIndexBuildsToo*/); } void checkUniqueIndexConstraints(OperationContext* opCtx, @@ -266,8 +269,7 @@ bool runCreateIndexes(OperationContext* opCtx, }; // Before potentially taking an exclusive database or collection lock, check if all indexes - // already exist while holding an intent lock. Only continue if new indexes need to be built - // and the collection or database should be re-locked in exclusive mode. + // already exist while holding an intent lock. { AutoGetCollection autoColl(opCtx, ns, MODE_IX); if (auto collection = autoColl.getCollection()) { @@ -634,7 +636,6 @@ bool runCreateIndexesWithCoordinator(OperationContext* opCtx, throw; } - result.append(kNumIndexesBeforeFieldName, stats.numIndexesBefore); result.append(kNumIndexesAfterFieldName, stats.numIndexesAfter); if (stats.numIndexesAfter == stats.numIndexesBefore) { @@ -681,11 +682,44 @@ public: const BSONObj& cmdObj, std::string& errmsg, BSONObjBuilder& result) override { - if (enableIndexBuildsCoordinatorForCreateIndexesCommand) { - return runCreateIndexesWithCoordinator( - opCtx, dbname, cmdObj, errmsg, result, false /*two phase build*/); + // If we encounter an IndexBuildAlreadyInProgress error for any of the requested index + // specs, then we will wait for the build(s) to finish before trying again. + const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbname, cmdObj)); + bool shouldLogMessageOnAlreadyBuildingError = true; + while (true) { + try { + if (enableIndexBuildsCoordinatorForCreateIndexesCommand) { + return runCreateIndexesWithCoordinator( + opCtx, dbname, cmdObj, errmsg, result, false /*two phase build*/); + } + return runCreateIndexes( + opCtx, dbname, cmdObj, errmsg, result, false /*two phase build*/); + } catch (const DBException& ex) { + if (ex.toStatus() != ErrorCodes::IndexBuildAlreadyInProgress) { + throw; + } + if (shouldLogMessageOnAlreadyBuildingError) { + auto bsonElem = cmdObj.getField(kIndexesFieldName); + log() + << "Received a request to create indexes: '" << bsonElem + << "', but found that at least one of the indexes is already being built, '" + << ex.toStatus() + << "'. This request will wait for the pre-existing index build to finish " + "before proceeding."; + shouldLogMessageOnAlreadyBuildingError = false; + } + // Unset the response fields so we do not write duplicate fields. + errmsg = ""; + result.resetToEmpty(); + // Reset the snapshot because we have released locks and may reacquire them again + // later. + opCtx->recoveryUnit()->abandonSnapshot(); + // This is a bit racy since we are not holding a lock across discovering an + // in-progress build and starting to listen for completion. It is good enough, + // however: we can only wait longer than needed, not less. + BackgroundOperation::waitUntilAnIndexBuildFinishes(opCtx, nss.ns()); + } } - return runCreateIndexes(opCtx, dbname, cmdObj, errmsg, result, false /*two phase build*/); } } cmdCreateIndex; diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 6f26850d256..a970944f3af 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -1138,7 +1138,8 @@ std::vector<BSONObj> IndexBuildsCoordinator::_addDefaultsAndFilterExistingIndexe uassertStatusOK(collection->addCollationDefaultsToIndexSpecsForCreate(opCtx, indexSpecs)); auto indexCatalog = collection->getIndexCatalog(); - auto filteredSpecs = indexCatalog->removeExistingIndexes(opCtx, specsWithCollationDefaults); + auto filteredSpecs = indexCatalog->removeExistingIndexes( + opCtx, specsWithCollationDefaults, true /*removeIndexBuildsToo*/); for (const BSONObj& spec : filteredSpecs) { if (spec[kUniqueFieldName].trueValue()) { |