diff options
-rw-r--r-- | jstests/noPassthrough/index_killop_after_stepdown.js | 97 | ||||
-rw-r--r-- | jstests/noPassthrough/index_stepdown_unique.js | 8 | ||||
-rw-r--r-- | src/mongo/db/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/catalog/SConscript | 13 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_build_oplog_entry.cpp | 139 | ||||
-rw-r--r-- | src/mongo/db/catalog/index_build_oplog_entry.h | 56 | ||||
-rw-r--r-- | src/mongo/db/commands/create_indexes.cpp | 46 | ||||
-rw-r--r-- | src/mongo/db/index_builds_coordinator.cpp | 140 | ||||
-rw-r--r-- | src/mongo/db/index_builds_coordinator.h | 35 | ||||
-rw-r--r-- | src/mongo/db/repl/SConscript | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/oplog.cpp | 260 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback.cpp | 113 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback_test.cpp | 10 |
13 files changed, 547 insertions, 375 deletions
diff --git a/jstests/noPassthrough/index_killop_after_stepdown.js b/jstests/noPassthrough/index_killop_after_stepdown.js new file mode 100644 index 00000000000..67841202c0c --- /dev/null +++ b/jstests/noPassthrough/index_killop_after_stepdown.js @@ -0,0 +1,97 @@ +/** + * Tests a race condition between a user interrupting an index build and the node stepping-down. The + * nature of this problem is that the stepping-down node is not able to replicate an abortIndexBuild + * oplog entry after the user kills the operation. The old primary will rely on the new primary to + * replicate a commitIndexBuild oplog entry after the takeover. + * + * @tags: [requires_replication] + */ +(function() { +"use strict"; + +load('jstests/noPassthrough/libs/index_build.js'); + +const rst = new ReplSetTest({ + nodes: [ + {}, + {}, + ] +}); +const nodes = rst.startSet(); +rst.initiate(); + +const primary = rst.getPrimary(); +const testDB = primary.getDB('test'); +const coll = testDB.getCollection('test'); + +assert.commandWorked(coll.insert({a: 1})); + +let res = assert.commandWorked(primary.adminCommand( + {configureFailPoint: 'hangAfterInitializingIndexBuild', mode: 'alwaysOn'})); +const hangAfterInitFailpointTimesEntered = res.count; + +res = assert.commandWorked(primary.adminCommand( + {configureFailPoint: 'hangBeforeIndexBuildAbortOnInterrupt', mode: 'alwaysOn'})); +const hangBeforeAbortFailpointTimesEntered = res.count; + +const createIdx = IndexBuildTest.startIndexBuild(primary, coll.getFullName(), {a: 1}); + +try { + assert.commandWorked(primary.adminCommand({ + waitForFailPoint: "hangAfterInitializingIndexBuild", + timesEntered: hangAfterInitFailpointTimesEntered + 1, + maxTimeMS: kDefaultWaitForFailPointTimeout + })); + + // When the index build starts, find its op id. This will be the op id of the client + // connection, not the thread pool task managed by IndexBuildsCoordinatorMongod. + const filter = {"desc": {$regex: /conn.*/}}; + const opId = IndexBuildTest.waitForIndexBuildToStart(testDB, coll.getName(), 'a_1', filter); + + // Kill the index build. + assert.commandWorked(testDB.killOp(opId)); + + // Let the index build continue running. + assert.commandWorked( + primary.adminCommand({configureFailPoint: 'hangAfterInitializingIndexBuild', mode: 'off'})); + + // Wait for the command thread to abort the index build. + assert.commandWorked(primary.adminCommand({ + waitForFailPoint: "hangBeforeIndexBuildAbortOnInterrupt", + timesEntered: hangBeforeAbortFailpointTimesEntered + 1, + maxTimeMS: kDefaultWaitForFailPointTimeout + })); + + // Step down the primary, preventing the index build from generating an abort oplog entry. + assert.commandWorked(testDB.adminCommand({replSetStepDown: 30, force: true})); +} finally { + assert.commandWorked( + primary.adminCommand({configureFailPoint: 'hangAfterInitializingIndexBuild', mode: 'off'})); + // Let the index build finish cleaning up. + assert.commandWorked(primary.adminCommand( + {configureFailPoint: 'hangBeforeIndexBuildAbortOnInterrupt', mode: 'off'})); +} + +const exitCode = createIdx({checkExitSuccess: false}); +assert.neq(0, exitCode, 'expected shell to exit abnormally due to index build being terminated'); + +// Wait for the index build to stop. +IndexBuildTest.waitForIndexBuildToStop(testDB); + +// With two phase index builds, a stepdown will not abort the index build, which should complete +// after a new node becomes primary. +rst.awaitReplication(); + +// The old primary, now secondary, should process the commitIndexBuild oplog entry. + +const secondaryColl = rst.getSecondary().getCollection(coll.getFullName()); +if (IndexBuildTest.supportsTwoPhaseIndexBuild(primary)) { + IndexBuildTest.assertIndexes(coll, 2, ['_id_', 'a_1'], [], {includeBuildUUIDs: true}); + IndexBuildTest.assertIndexes(secondaryColl, 2, ['_id_', 'a_1'], [], {includeBuildUUIDs: true}); +} else { + IndexBuildTest.assertIndexes(coll, 1, ['_id_'], [], {includeBuildUUIDs: true}); + IndexBuildTest.assertIndexes(secondaryColl, 1, ['_id_'], [], {includeBuildUUIDs: true}); +} + +rst.stopSet(); +})(); diff --git a/jstests/noPassthrough/index_stepdown_unique.js b/jstests/noPassthrough/index_stepdown_unique.js index ada233c5f45..02be8163efe 100644 --- a/jstests/noPassthrough/index_stepdown_unique.js +++ b/jstests/noPassthrough/index_stepdown_unique.js @@ -12,13 +12,7 @@ load('jstests/noPassthrough/libs/index_build.js'); const rst = new ReplSetTest({ nodes: [ {}, - { - // Disallow elections on secondary. - rsConfig: { - priority: 0, - votes: 0, - }, - }, + {}, ] }); const nodes = rst.startSet(); diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 91d278dd5c7..2be1bdca9af 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -801,6 +801,7 @@ env.Library( 'index_build_entry_helpers', 'server_options_core', '$BUILD_DIR/mongo/db/catalog/index_build_entry_idl', + '$BUILD_DIR/mongo/db/catalog/index_build_oplog_entry', '$BUILD_DIR/mongo/db/catalog/collection', '$BUILD_DIR/mongo/db/catalog/collection_catalog', '$BUILD_DIR/mongo/db/concurrency/lock_manager', diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript index c122e03a271..e5c6db5bf68 100644 --- a/src/mongo/db/catalog/SConscript +++ b/src/mongo/db/catalog/SConscript @@ -124,6 +124,19 @@ env.Library( ) env.Library( + target='index_build_oplog_entry', + source=[ + "index_build_oplog_entry.cpp", + ], + LIBDEPS=[ + '$BUILD_DIR/mongo/base', + "$BUILD_DIR/mongo/bson/util/bson_extract", + "$BUILD_DIR/mongo/db/repl/oplog_entry", + "$BUILD_DIR/mongo/rpc/command_status", + ], +) + +env.Library( target='index_key_validate', source=[ "index_key_validate.cpp", diff --git a/src/mongo/db/catalog/index_build_oplog_entry.cpp b/src/mongo/db/catalog/index_build_oplog_entry.cpp new file mode 100644 index 00000000000..4512f43af9b --- /dev/null +++ b/src/mongo/db/catalog/index_build_oplog_entry.cpp @@ -0,0 +1,139 @@ +/** + * Copyright (C) 2020-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/db/catalog/index_build_oplog_entry.h" + +#include "mongo/bson/util/bson_extract.h" +#include "mongo/logger/redaction.h" +#include "mongo/rpc/get_status_from_command_result.h" + +namespace mongo { +StatusWith<IndexBuildOplogEntry> IndexBuildOplogEntry::parse(const repl::OplogEntry& entry) { + // Example 'o' field which takes the same form for all three oplog entries. + // { + // < "startIndexBuild" | "commitIndexBuild" | "abortIndexBuild" > : "coll", + // "indexBuildUUID" : <UUID>, + // "indexes" : [ + // { + // "key" : { + // "x" : 1 + // }, + // "name" : "x_1", + // ... + // }, + // { + // "key" : { + // "k" : 1 + // }, + // "name" : "k_1", + // ... + // } + // ], + // "cause" : <Object> // Only required for 'abortIndexBuild'. + // } + // + // + // Ensure the collection name is specified + invariant(entry.getOpType() == repl::OpTypeEnum::kCommand); + + auto commandType = entry.getCommandType(); + invariant(commandType == repl::OplogEntry::CommandType::kStartIndexBuild || + commandType == repl::OplogEntry::CommandType::kCommitIndexBuild || + commandType == repl::OplogEntry::CommandType::kAbortIndexBuild); + + BSONObj obj = entry.getObject(); + BSONElement first = obj.firstElement(); + auto commandName = first.fieldNameStringData(); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << commandName << " value must be a string", + first.type() == mongo::String); + + auto buildUUIDElem = obj.getField("indexBuildUUID"); + if (buildUUIDElem.eoo()) { + return {ErrorCodes::BadValue, str::stream() << "Missing required field 'indexBuildUUID'"}; + } + auto swBuildUUID = UUID::parse(buildUUIDElem); + if (!swBuildUUID.isOK()) { + return swBuildUUID.getStatus().withContext("Error parsing 'indexBuildUUID'"); + } + + auto indexesElem = obj.getField("indexes"); + if (indexesElem.eoo()) { + return {ErrorCodes::BadValue, str::stream() << "Missing required field 'indexes'"}; + } + + if (indexesElem.type() != Array) { + return {ErrorCodes::BadValue, + str::stream() << "Field 'indexes' must be an array of index spec objects"}; + } + + std::vector<std::string> indexNames; + std::vector<BSONObj> indexSpecs; + for (auto& indexElem : indexesElem.Array()) { + if (!indexElem.isABSONObj()) { + return {ErrorCodes::BadValue, + str::stream() << "Element of 'indexes' must be an object"}; + } + std::string indexName; + auto status = bsonExtractStringField(indexElem.Obj(), "name", &indexName); + if (!status.isOK()) { + return status.withContext("Error extracting 'name' from index spec"); + } + indexNames.push_back(indexName); + indexSpecs.push_back(indexElem.Obj().getOwned()); + } + + // Get the reason this index build was aborted on the primary. + boost::optional<Status> cause; + if (repl::OplogEntry::CommandType::kAbortIndexBuild == commandType) { + auto causeElem = obj.getField("cause"); + if (causeElem.eoo()) { + return {ErrorCodes::BadValue, "Missing required field 'cause'."}; + } + if (causeElem.type() != Object) { + return {ErrorCodes::BadValue, "Field 'cause' must be an object."}; + } + auto causeStatusObj = causeElem.Obj(); + cause = getStatusFromCommandResult(causeStatusObj); + } + + auto collUUID = entry.getUuid(); + invariant(collUUID, str::stream() << redact(entry.getRaw())); + + return IndexBuildOplogEntry{*collUUID, + commandType, + commandName.toString(), + swBuildUUID.getValue(), + indexNames, + indexSpecs, + cause}; +} +} // namespace mongo diff --git a/src/mongo/db/catalog/index_build_oplog_entry.h b/src/mongo/db/catalog/index_build_oplog_entry.h new file mode 100644 index 00000000000..284afbc669e --- /dev/null +++ b/src/mongo/db/catalog/index_build_oplog_entry.h @@ -0,0 +1,56 @@ +/** + * Copyright (C) 2020-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#pragma once + +#include <vector> + +#include "mongo/base/status_with.h" +#include "mongo/db/repl/oplog_entry.h" +#include "mongo/util/uuid.h" + +namespace mongo { + +class IndexBuildOplogEntry { +public: + /** + * Parses an oplog entry for "startIndexBuild", "commitIndexBuild", or "abortIndexBuild". + */ + static StatusWith<IndexBuildOplogEntry> parse(const repl::OplogEntry& entry); + + UUID collUUID; + repl::OplogEntry::CommandType commandType; + std::string commandName; + UUID buildUUID; + std::vector<std::string> indexNames; + std::vector<BSONObj> indexSpecs; + boost::optional<Status> cause; +}; + +} // namespace mongo diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index 55b4857c1d7..0213275eb86 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -77,6 +77,7 @@ MONGO_FAIL_POINT_DEFINE(createIndexesWriteConflict); // This failpoint causes createIndexes with an implicit collection creation to hang before the // collection is created. MONGO_FAIL_POINT_DEFINE(hangBeforeCreateIndexesCollectionCreate); +MONGO_FAIL_POINT_DEFINE(hangBeforeIndexBuildAbortOnInterrupt); constexpr auto kIndexesFieldName = "indexes"_sd; constexpr auto kCommandName = "createIndexes"_sd; @@ -491,8 +492,13 @@ BSONObj runCreateIndexesOnNewCollection(OperationContext* opCtx, const int numIndexesBefore = IndexBuildsCoordinator::getNumIndexesTotal(opCtx, collection); auto filteredSpecs = IndexBuildsCoordinator::prepareSpecListForCreate(opCtx, collection, ns, specs); - IndexBuildsCoordinator::createIndexesOnEmptyCollection( - opCtx, collection->uuid(), filteredSpecs, false); + // It's possible for 'filteredSpecs' to be empty if we receive a createIndexes request for the + // _id index and also create the collection implicitly. By this point, the _id index has already + // been created, and there is no more work to be done. + if (!filteredSpecs.empty()) { + IndexBuildsCoordinator::createIndexesOnEmptyCollection( + opCtx, collection->uuid(), filteredSpecs, false); + } const int numIndexesAfter = IndexBuildsCoordinator::getNumIndexesTotal(opCtx, collection); @@ -865,14 +871,32 @@ bool runCreateIndexesWithCoordinator(OperationContext* opCtx, } catch (const ExceptionForCat<ErrorCategory::Interruption>& interruptionEx) { log() << "Index build interrupted: " << buildUUID << ": " << interruptionEx; - // If this node is no longer a primary, the index build will continue to run in the - // background and will complete when this node receives a commitIndexBuild oplog entry - // from the new primary. + hangBeforeIndexBuildAbortOnInterrupt.pauseWhileSet(); - if (indexBuildsCoord->supportsTwoPhaseIndexBuild() && - ErrorCodes::InterruptedDueToReplStateChange == interruptionEx.code()) { - log() << "Index build continuing in background: " << buildUUID; - throw; + boost::optional<Lock::GlobalLock> globalLock; + if (IndexBuildProtocol::kTwoPhase == protocol) { + // If this node is no longer a primary, the index build will continue to run in the + // background and will complete when this node receives a commitIndexBuild oplog + // entry from the new primary. + if (ErrorCodes::InterruptedDueToReplStateChange == interruptionEx.code()) { + log() << "Index build continuing in background: " << buildUUID; + throw; + } + + // If we are using two-phase index builds and are no longer primary after receiving + // an interrupt, we cannot replicate an abortIndexBuild oplog entry. Rely on the new + // primary to finish the index build. Acquire the global lock to check the + // replication state and to prevent any state transitions from happening while + // aborting the index build. + UninterruptibleLockGuard noInterrupt(opCtx->lockState()); + globalLock.emplace(opCtx, MODE_IS); + if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, ns)) { + uassertStatusOK( + {ErrorCodes::NotMaster, + str::stream() + << "Unable to abort index build because we are no longer primary: " + << buildUUID}); + } } // It is unclear whether the interruption originated from the current opCtx instance @@ -880,7 +904,7 @@ bool runCreateIndexesWithCoordinator(OperationContext* opCtx, // independently of this command invocation. We'll defensively abort the index build // with the assumption that if the index build was already in the midst of tearing down, // this be a no-op. - indexBuildsCoord->abortIndexBuildByBuildUUID( + indexBuildsCoord->abortIndexBuildByBuildUUIDNoWait( opCtx, buildUUID, str::stream() << "Index build interrupted: " << buildUUID << ": " @@ -900,7 +924,7 @@ bool runCreateIndexesWithCoordinator(OperationContext* opCtx, throw; } - indexBuildsCoord->abortIndexBuildByBuildUUID( + indexBuildsCoord->abortIndexBuildByBuildUUIDNoWait( opCtx, buildUUID, str::stream() << "Index build interrupted due to change in replication state: " diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp index 1a3eec931fa..18ccf79077d 100644 --- a/src/mongo/db/index_builds_coordinator.cpp +++ b/src/mongo/db/index_builds_coordinator.cpp @@ -524,16 +524,93 @@ void IndexBuildsCoordinator::abortDatabaseIndexBuilds(StringData db, const std:: dbIndexBuilds->waitUntilNoIndexBuildsRemain(lk); } -void IndexBuildsCoordinator::signalCommitAndWait(OperationContext* opCtx, const UUID& buildUUID) { +namespace { +NamespaceString getNsFromUUID(OperationContext* opCtx, const UUID& uuid) { + auto& catalog = CollectionCatalog::get(opCtx); + auto nss = catalog.lookupNSSByUUID(opCtx, uuid); + uassert(ErrorCodes::NamespaceNotFound, "No namespace with UUID " + uuid.toString(), nss); + return *nss; +} +} // namespace + +void IndexBuildsCoordinator::applyStartIndexBuild(OperationContext* opCtx, + const IndexBuildOplogEntry& oplogEntry) { + const auto collUUID = oplogEntry.collUUID; + const auto nss = getNsFromUUID(opCtx, collUUID); + + IndexBuildsCoordinator::IndexBuildOptions indexBuildOptions; + invariant(!indexBuildOptions.commitQuorum); + indexBuildOptions.replSetAndNotPrimaryAtStart = true; + + auto indexBuildsCoord = IndexBuildsCoordinator::get(opCtx); + uassertStatusOK( + indexBuildsCoord + ->startIndexBuild(opCtx, + nss.db().toString(), + collUUID, + oplogEntry.indexSpecs, + oplogEntry.buildUUID, + /* This oplog entry is only replicated for two-phase index builds */ + IndexBuildProtocol::kTwoPhase, + indexBuildOptions) + .getStatus()); +} + +void IndexBuildsCoordinator::applyCommitIndexBuild(OperationContext* opCtx, + const IndexBuildOplogEntry& oplogEntry) { + const auto collUUID = oplogEntry.collUUID; + const auto nss = getNsFromUUID(opCtx, collUUID); + const auto& buildUUID = oplogEntry.buildUUID; + updateCurOpForCommitOrAbort(opCtx, kCommitIndexBuildFieldName, buildUUID); - auto replState = uassertStatusOK(_getIndexBuild(buildUUID)); + uassert(31417, + str::stream() + << "No commit timestamp set while applying commitIndexBuild operation. Build UUID: " + << buildUUID, + !opCtx->recoveryUnit()->getCommitTimestamp().isNull()); + + auto indexBuildsCoord = IndexBuildsCoordinator::get(opCtx); + auto swReplState = indexBuildsCoord->_getIndexBuild(buildUUID); + if (swReplState == ErrorCodes::NoSuchKey) { + // If the index build was not found, we must restart the build. For some reason the index + // build has already been aborted on this node. This is possible in certain infrequent race + // conditions with stepdown, shutdown, and user interruption. + log() << "Could not find an active index build with UUID " << buildUUID + << " while processing a commitIndexBuild oplog entry. Restarting the index build on " + "collection " + << nss << " (" << collUUID << ") at optime " + << opCtx->recoveryUnit()->getCommitTimestamp(); + + IndexBuildsCoordinator::IndexBuildOptions indexBuildOptions; + indexBuildOptions.replSetAndNotPrimaryAtStart = true; + // This spawns a new thread and returns immediately. + auto fut = uassertStatusOK(indexBuildsCoord->startIndexBuild( + opCtx, + nss.db().toString(), + collUUID, + oplogEntry.indexSpecs, + buildUUID, + /* This oplog entry is only replicated for two-phase index builds */ + IndexBuildProtocol::kTwoPhase, + indexBuildOptions)); + + // In certain optimized cases that return early, the future will already be set, and the + // index build will already have been torn-down. Any subsequent calls to look up the index + // build will fail immediately without any error information. + if (fut.isReady()) { + // Throws if there were errors building the index. + fut.get(); + return; + } + } + + auto replState = uassertStatusOK(indexBuildsCoord->_getIndexBuild(buildUUID)); { stdx::unique_lock<Latch> lk(replState->mutex); replState->isCommitReady = true; replState->commitTimestamp = opCtx->recoveryUnit()->getCommitTimestamp(); - invariant(!replState->commitTimestamp.isNull(), buildUUID.toString()); replState->condVar.notify_all(); } auto fut = replState->sharedPromise.getFuture(); @@ -543,42 +620,58 @@ void IndexBuildsCoordinator::signalCommitAndWait(OperationContext* opCtx, const fut.get(); } -void IndexBuildsCoordinator::signalAbortAndWait(OperationContext* opCtx, - const UUID& buildUUID, - const std::string& reason) noexcept { - updateCurOpForCommitOrAbort(opCtx, kAbortIndexBuildFieldName, buildUUID); +void IndexBuildsCoordinator::applyAbortIndexBuild(OperationContext* opCtx, + const IndexBuildOplogEntry& oplogEntry) { + const auto collUUID = oplogEntry.collUUID; + const auto nss = getNsFromUUID(opCtx, collUUID); + const auto& buildUUID = oplogEntry.buildUUID; - abortIndexBuildByBuildUUID(opCtx, buildUUID, reason); + updateCurOpForCommitOrAbort(opCtx, kCommitIndexBuildFieldName, buildUUID); - // Because we replicate abort oplog entries for single-phase builds, it is possible to receive - // an abort for a non-existent index build. Abort should always succeed, so suppress the error. - auto replStateResult = _getIndexBuild(buildUUID); - if (!replStateResult.isOK()) { - log() << "ignoring error while aborting index build " << buildUUID << ": " - << replStateResult.getStatus(); + invariant(oplogEntry.cause); + auto indexBuildsCoord = IndexBuildsCoordinator::get(opCtx); + indexBuildsCoord->abortIndexBuildByBuildUUID( + opCtx, + buildUUID, + str::stream() << "abortIndexBuild oplog entry encountered: " << *oplogEntry.cause); +} + +void IndexBuildsCoordinator::abortIndexBuildByBuildUUID(OperationContext* opCtx, + const UUID& buildUUID, + const std::string& reason) { + if (!abortIndexBuildByBuildUUIDNoWait(opCtx, buildUUID, reason)) { return; } - auto replState = replStateResult.getValue(); + auto replState = invariant(_getIndexBuild(buildUUID)); + auto fut = replState->sharedPromise.getFuture(); - log() << "Index build joined after abort: " << buildUUID << ": " << fut.waitNoThrow(opCtx); + log() << "Index build joined after abort: " << buildUUID << ": " << fut.waitNoThrow(); } -void IndexBuildsCoordinator::abortIndexBuildByBuildUUID(OperationContext* opCtx, - const UUID& buildUUID, - const std::string& reason) { +bool IndexBuildsCoordinator::abortIndexBuildByBuildUUIDNoWait(OperationContext* opCtx, + const UUID& buildUUID, + const std::string& reason) { _indexBuildsManager.abortIndexBuild(buildUUID, reason); + // It is possible to receive an abort for a non-existent index build. Abort should always + // succeed, so suppress the error. auto replStateResult = _getIndexBuild(buildUUID); - if (replStateResult.isOK()) { - auto replState = replStateResult.getValue(); + if (!replStateResult.isOK()) { + log() << "ignoring error while aborting index build " << buildUUID << ": " + << replStateResult.getStatus(); + return false; + } + auto replState = replStateResult.getValue(); + { stdx::unique_lock<Latch> lk(replState->mutex); replState->aborted = true; replState->abortTimestamp = opCtx->recoveryUnit()->getCommitTimestamp(); replState->abortReason = reason; replState->condVar.notify_all(); } + return true; } /** @@ -610,7 +703,9 @@ void IndexBuildsCoordinator::onStepUp(OperationContext* opCtx) { // oplog entries, and consequently does not have a timestamp to delete the index from // the durable catalog. This abort will replicate to the old primary, now secondary, to // abort the build. - abortIndexBuildByBuildUUID( + // Do not wait for the index build to exit, because it may reacquire locks that are not + // available until stepUp completes. + abortIndexBuildByBuildUUIDNoWait( opCtx, replState->buildUUID, "unique indexes do not support failover"); return; } @@ -850,6 +945,7 @@ void IndexBuildsCoordinator::createIndexesOnEmptyCollection(OperationContext* op invariant(collection, str::stream() << collectionUUID); invariant(0U == collection->numRecords(opCtx), str::stream() << collectionUUID); + invariant(!specs.empty(), str::stream() << collectionUUID); auto nss = collection->ns(); invariant( diff --git a/src/mongo/db/index_builds_coordinator.h b/src/mongo/db/index_builds_coordinator.h index 292bdff3eb3..042c172ebca 100644 --- a/src/mongo/db/index_builds_coordinator.h +++ b/src/mongo/db/index_builds_coordinator.h @@ -36,6 +36,7 @@ #include "mongo/base/string_data.h" #include "mongo/db/catalog/collection_options.h" #include "mongo/db/catalog/commit_quorum_options.h" +#include "mongo/db/catalog/index_build_oplog_entry.h" #include "mongo/db/catalog/index_builds.h" #include "mongo/db/catalog/index_builds_manager.h" #include "mongo/db/collection_index_builds_tracker.h" @@ -43,6 +44,7 @@ #include "mongo/db/database_index_builds_tracker.h" #include "mongo/db/namespace_string.h" #include "mongo/db/repair_database.h" +#include "mongo/db/repl/oplog_entry.h" #include "mongo/db/repl_index_build_state.h" #include "mongo/db/storage/durable_catalog.h" #include "mongo/platform/mutex.h" @@ -160,18 +162,23 @@ public: RepairData repair); /** - * Signals the index build identified by 'buildUUID' to commit, and waits for its thread to - * complete. Throws if there were any errors building the index. + * Apply a 'startIndexBuild' oplog entry. Returns when the index build thread has started and + * performed the initial ready:false write. Throws if there were any errors building the index. */ - void signalCommitAndWait(OperationContext* opCtx, const UUID& buildUUID); + void applyStartIndexBuild(OperationContext* opCtx, const IndexBuildOplogEntry& entry); /** - * Signals the index build identified by 'buildUUID' to abort, and waits for its thread to - * complete. + * Apply a 'commitIndexBuild' oplog entry. If no index build is found, starts an index build + * with the provided information. In all cases, waits until the index build commits and the + * thread exits. Throws if there were any errors building the index. */ - void signalAbortAndWait(OperationContext* opCtx, - const UUID& buildUUID, - const std::string& reason) noexcept; + void applyCommitIndexBuild(OperationContext* opCtx, const IndexBuildOplogEntry& entry); + + /** + * Apply an 'abortIndexBuild' oplog entry. Waits until the index build aborts and the + * thread exits. Throws if there were any errors aborting the index. + */ + void applyAbortIndexBuild(OperationContext* opCtx, const IndexBuildOplogEntry& entry); /** * Waits for all index builds to stop after they have been interrupted during shutdown. @@ -227,13 +234,21 @@ public: void abortDatabaseIndexBuilds(StringData db, const std::string& reason); /** - * Aborts a given index build by index build UUID. + * Aborts an index build by index build UUID. Returns when the index build thread exits. */ void abortIndexBuildByBuildUUID(OperationContext* opCtx, const UUID& buildUUID, const std::string& reason); /** + * Aborts an index build by index build UUID. Does not wait for the index build thread to + * exit. Returns true if an index build was aborted. + */ + bool abortIndexBuildByBuildUUIDNoWait(OperationContext* opCtx, + const UUID& buildUUID, + const std::string& reason); + + /** * Invoked when the node enters the primary state. * Unblocks index builds that have been waiting to commit/abort during the secondary state. */ @@ -602,7 +617,7 @@ protected: RepairData repair) noexcept; /** - * Looks up active index build by UUID. + * Looks up active index build by UUID. Returns NoSuchKey if the build does not exist. */ StatusWith<std::shared_ptr<ReplIndexBuildState>> _getIndexBuild(const UUID& buildUUID) const; diff --git a/src/mongo/db/repl/SConscript b/src/mongo/db/repl/SConscript index 0b23d85972b..a637f4a7e05 100644 --- a/src/mongo/db/repl/SConscript +++ b/src/mongo/db/repl/SConscript @@ -47,6 +47,7 @@ env.Library( '$BUILD_DIR/mongo/db/background', '$BUILD_DIR/mongo/db/catalog/catalog_helpers', '$BUILD_DIR/mongo/db/catalog/database_holder', + '$BUILD_DIR/mongo/db/catalog/index_build_oplog_entry', '$BUILD_DIR/mongo/db/catalog/multi_index_block', '$BUILD_DIR/mongo/db/commands/feature_compatibility_parsers', '$BUILD_DIR/mongo/db/db_raii', @@ -405,6 +406,9 @@ env.Library( '$BUILD_DIR/mongo/db/dbhelpers', '$BUILD_DIR/mongo/db/query_exec', ], + LIBDEPS_PRIVATE=[ + '$BUILD_DIR/mongo/db/catalog/index_build_oplog_entry', + ], ) env.Library( diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp index 51c61a7a182..3578b6eb7e2 100644 --- a/src/mongo/db/repl/oplog.cpp +++ b/src/mongo/db/repl/oplog.cpp @@ -156,79 +156,6 @@ void setOplogCollectionName(ServiceContext* service) { LocalOplogInfo::get(service)->setOplogCollectionName(service); } -/** - * Parse the given BSON array of BSON into a vector of BSON. - */ -StatusWith<std::vector<BSONObj>> parseBSONSpecsIntoVector(const BSONElement& bsonArrayElem, - const NamespaceString& nss) { - invariant(bsonArrayElem.type() == Array); - std::vector<BSONObj> vec; - for (auto& bsonElem : bsonArrayElem.Array()) { - if (bsonElem.type() != BSONType::Object) { - return {ErrorCodes::TypeMismatch, - str::stream() << "The elements of '" << bsonArrayElem.fieldName() - << "' array must be objects, but found " - << typeName(bsonElem.type())}; - } - vec.emplace_back(bsonElem.Obj().getOwned()); - } - return vec; -} - -Status startIndexBuild(OperationContext* opCtx, - const NamespaceString& nss, - const UUID& collUUID, - const UUID& indexBuildUUID, - const BSONElement& indexesElem, - OplogApplication::Mode mode) { - auto statusWithIndexes = parseBSONSpecsIntoVector(indexesElem, nss); - if (!statusWithIndexes.isOK()) { - return statusWithIndexes.getStatus(); - } - - IndexBuildsCoordinator::IndexBuildOptions indexBuildOptions; - invariant(!indexBuildOptions.commitQuorum); - indexBuildOptions.replSetAndNotPrimaryAtStart = true; - - // We don't pass in a commit quorum here because secondary nodes don't have any knowledge of it. - return IndexBuildsCoordinator::get(opCtx) - ->startIndexBuild(opCtx, - nss.db().toString(), - collUUID, - statusWithIndexes.getValue(), - indexBuildUUID, - /* This oplog entry is only replicated for two-phase index builds */ - IndexBuildProtocol::kTwoPhase, - indexBuildOptions) - .getStatus(); -} - -Status commitIndexBuild(OperationContext* opCtx, - const NamespaceString& nss, - const UUID& indexBuildUUID, - const BSONElement& indexesElem, - OplogApplication::Mode mode) { - auto statusWithIndexes = parseBSONSpecsIntoVector(indexesElem, nss); - if (!statusWithIndexes.isOK()) { - return statusWithIndexes.getStatus(); - } - auto indexBuildsCoord = IndexBuildsCoordinator::get(opCtx); - indexBuildsCoord->signalCommitAndWait(opCtx, indexBuildUUID); - return Status::OK(); -} - -Status abortIndexBuild(OperationContext* opCtx, - const UUID& indexBuildUUID, - const Status& cause, - OplogApplication::Mode mode) { - // Wait until the index build finishes aborting. - IndexBuildsCoordinator::get(opCtx)->signalAbortAndWait( - opCtx, - indexBuildUUID, - str::stream() << "abortIndexBuild oplog entry encountered: " << cause); - return Status::OK(); -} - void createIndexForApplyOps(OperationContext* opCtx, const BSONObj& indexSpec, const NamespaceString& indexNss, @@ -771,188 +698,59 @@ const StringMap<ApplyOpMetadata> kOpsMap = { ErrorCodes::NamespaceNotFound}}}, {"startIndexBuild", {[](OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode) -> Status { - // { - // "startIndexBuild" : "coll", - // "indexBuildUUID" : <UUID>, - // "indexes" : [ - // { - // "key" : { - // "x" : 1 - // }, - // "name" : "x_1", - // "v" : 2 - // }, - // { - // "key" : { - // "k" : 1 - // }, - // "name" : "k_1", - // "v" : 2 - // } - // ] - // } - if (OplogApplication::Mode::kApplyOpsCmd == mode) { return {ErrorCodes::CommandNotSupported, "The startIndexBuild operation is not supported in applyOps mode"}; } - const auto& ui = entry.getUuid(); - const auto& cmd = entry.getObject(); - const NamespaceString nss(extractNsFromUUIDorNs(opCtx, entry.getNss(), ui, cmd)); - - auto buildUUIDElem = cmd.getField("indexBuildUUID"); - uassert(ErrorCodes::BadValue, - "Error parsing 'startIndexBuild' oplog entry, missing required field " - "'indexBuildUUID'.", - !buildUUIDElem.eoo()); - UUID indexBuildUUID = uassertStatusOK(UUID::parse(buildUUIDElem)); - - auto indexesElem = cmd.getField("indexes"); - uassert(ErrorCodes::BadValue, - "Error parsing 'startIndexBuild' oplog entry, missing required field 'indexes'.", - !indexesElem.eoo()); - uassert(ErrorCodes::BadValue, - "Error parsing 'startIndexBuild' oplog entry, field 'indexes' must be an array.", - indexesElem.type() == Array); - - uassert(ErrorCodes::BadValue, - "Error parsing 'startIndexBuild' oplog entry, missing required field 'uuid'.", - ui); - auto collUUID = ui.get(); - - if (IndexBuildsCoordinator::supportsTwoPhaseIndexBuild()) { - return startIndexBuild(opCtx, nss, collUUID, indexBuildUUID, indexesElem, mode); + if (!IndexBuildsCoordinator::supportsTwoPhaseIndexBuild()) { + return Status::OK(); } + auto swOplogEntry = IndexBuildOplogEntry::parse(entry); + if (!swOplogEntry.isOK()) { + return swOplogEntry.getStatus().withContext( + "Error parsing 'startIndexBuild' oplog entry"); + } + + IndexBuildsCoordinator::get(opCtx)->applyStartIndexBuild(opCtx, swOplogEntry.getValue()); return Status::OK(); }, - {ErrorCodes::NamespaceNotFound}}}, + {ErrorCodes::IndexAlreadyExists, + ErrorCodes::IndexBuildAlreadyInProgress, + ErrorCodes::NamespaceNotFound}}}, {"commitIndexBuild", {[](OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode) -> Status { - // { - // "commitIndexBuild" : "coll", - // "indexBuildUUID" : <UUID>, - // "indexes" : [ - // { - // "key" : { - // "x" : 1 - // }, - // "name" : "x_1", - // "v" : 2 - // }, - // { - // "key" : { - // "k" : 1 - // }, - // "name" : "k_1", - // "v" : 2 - // } - // ] - // } - if (OplogApplication::Mode::kApplyOpsCmd == mode) { return {ErrorCodes::CommandNotSupported, "The commitIndexBuild operation is not supported in applyOps mode"}; } - const auto& cmd = entry.getObject(); - // Ensure the collection name is specified - BSONElement first = cmd.firstElement(); - invariant(first.fieldNameStringData() == "commitIndexBuild"); - uassert(ErrorCodes::InvalidNamespace, - "commitIndexBuild value must be a string", - first.type() == mongo::String); - - // May throw NamespaceNotFound exception on a non-existent collection, especially if two - // phase index builds are not enabled. - const NamespaceString nss( - extractNsFromUUIDorNs(opCtx, entry.getNss(), entry.getUuid(), cmd)); - - auto buildUUIDElem = cmd.getField("indexBuildUUID"); - uassert(ErrorCodes::BadValue, - "Error parsing 'commitIndexBuild' oplog entry, missing required field " - "'indexBuildUUID'.", - !buildUUIDElem.eoo()); - UUID indexBuildUUID = uassertStatusOK(UUID::parse(buildUUIDElem)); - - auto indexesElem = cmd.getField("indexes"); - uassert(ErrorCodes::BadValue, - "Error parsing 'commitIndexBuild' oplog entry, missing required field 'indexes'.", - !indexesElem.eoo()); - uassert(ErrorCodes::BadValue, - "Error parsing 'commitIndexBuild' oplog entry, field 'indexes' must be an array.", - indexesElem.type() == Array); - - return commitIndexBuild(opCtx, nss, indexBuildUUID, indexesElem, mode); + auto swOplogEntry = IndexBuildOplogEntry::parse(entry); + if (!swOplogEntry.isOK()) { + return swOplogEntry.getStatus().withContext( + "Error parsing 'commitIndexBuild' oplog entry"); + } + IndexBuildsCoordinator::get(opCtx)->applyCommitIndexBuild(opCtx, swOplogEntry.getValue()); + return Status::OK(); }, - {ErrorCodes::NamespaceNotFound, ErrorCodes::NoSuchKey}}}, + {ErrorCodes::IndexAlreadyExists, + ErrorCodes::IndexBuildAlreadyInProgress, + ErrorCodes::NamespaceNotFound}}}, {"abortIndexBuild", {[](OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode) -> Status { - // { - // "abortIndexBuild" : "coll", - // "indexBuildUUID" : <UUID>, - // "indexes" : [ - // { - // "key" : { - // "x" : 1 - // }, - // "name" : "x_1", - // "v" : 2 - // }, - // { - // "key" : { - // "k" : 1 - // }, - // "name" : "k_1", - // "v" : 2 - // } - // ] - // } - if (OplogApplication::Mode::kApplyOpsCmd == mode) { return {ErrorCodes::CommandNotSupported, "The abortIndexBuild operation is not supported in applyOps mode"}; } - const auto& cmd = entry.getObject(); - // Ensure that the first element is the 'abortIndexBuild' field. - BSONElement first = cmd.firstElement(); - invariant(first.fieldNameStringData() == "abortIndexBuild"); - uassert(ErrorCodes::InvalidNamespace, - "abortIndexBuild value must be a string specifying the collection name", - first.type() == mongo::String); - - auto buildUUIDElem = cmd.getField("indexBuildUUID"); - uassert(ErrorCodes::BadValue, - "Error parsing 'abortIndexBuild' oplog entry, missing required field " - "'indexBuildUUID'.", - !buildUUIDElem.eoo()); - UUID indexBuildUUID = uassertStatusOK(UUID::parse(buildUUIDElem)); - - // We require the indexes field to ensure that rollback via refetch knows the appropriate - // indexes to rebuild. - auto indexesElem = cmd.getField("indexes"); - uassert(ErrorCodes::BadValue, - "Error parsing 'abortIndexBuild' oplog entry, missing required field 'indexes'.", - !indexesElem.eoo()); - uassert(ErrorCodes::BadValue, - "Error parsing 'abortIndexBuild' oplog entry, field 'indexes' must be an array of " - "index names.", - indexesElem.type() == Array); - - // Get the reason this index build was aborted on the primary. - auto causeElem = cmd.getField("cause"); - uassert(ErrorCodes::BadValue, - "Error parsing 'abortIndexBuild' oplog entry, missing required field 'cause'.", - !causeElem.eoo()); - uassert(ErrorCodes::BadValue, - "Error parsing 'abortIndexBuild' oplog entry, field 'cause' must be an object.", - causeElem.type() == Object); - auto causeStatusObj = causeElem.Obj(); - auto cause = getStatusFromCommandResult(causeStatusObj); - - return abortIndexBuild(opCtx, indexBuildUUID, cause, mode); + auto swOplogEntry = IndexBuildOplogEntry::parse(entry); + if (!swOplogEntry.isOK()) { + return swOplogEntry.getStatus().withContext( + "Error parsing 'abortIndexBuild' oplog entry"); + } + IndexBuildsCoordinator::get(opCtx)->applyAbortIndexBuild(opCtx, swOplogEntry.getValue()); + return Status::OK(); }}}, {"collMod", {[](OperationContext* opCtx, const OplogEntry& entry, OplogApplication::Mode mode) -> Status { diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp index cf972d3065f..35f6901edcc 100644 --- a/src/mongo/db/repl/rs_rollback.cpp +++ b/src/mongo/db/repl/rs_rollback.cpp @@ -42,6 +42,7 @@ #include "mongo/db/catalog/collection_catalog.h" #include "mongo/db/catalog/database_holder.h" #include "mongo/db/catalog/document_validation.h" +#include "mongo/db/catalog/index_build_oplog_entry.h" #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/catalog/rename_collection.h" #include "mongo/db/catalog_raii.h" @@ -220,78 +221,6 @@ Status FixUpInfo::recordDropTargetInfo(const BSONElement& dropTarget, return Status::OK(); } -namespace { - -typedef struct { - UUID buildUUID; - std::vector<std::string> indexNames; - std::vector<BSONObj> indexSpecs; -} IndexBuildOplogEntry; - -// Parses an oplog entry for "startIndexBuild", "commitIndexBuild", or "abortIndexBuild". -StatusWith<IndexBuildOplogEntry> parseIndexBuildOplogObject(const BSONObj& obj) { - // Example object which takes the same form for all three oplog entries. - // { - // < "startIndexBuild" | "commitIndexBuild" | "abortIndexBuild" > : "coll", - // "indexBuildUUID" : <UUID>, - // "indexes" : [ - // { - // "key" : { - // "x" : 1 - // }, - // "name" : "x_1", - // "v" : 2 - // }, - // { - // "key" : { - // "k" : 1 - // }, - // "name" : "k_1", - // "v" : 2 - // } - // ] - // } - // - // - auto buildUUIDElem = obj.getField("indexBuildUUID"); - if (buildUUIDElem.eoo()) { - return {ErrorCodes::BadValue, str::stream() << "Missing required field 'indexBuildUUID'"}; - } - auto swBuildUUID = UUID::parse(buildUUIDElem); - if (!swBuildUUID.isOK()) { - return swBuildUUID.getStatus().withContext("Error parsing 'indexBuildUUID'"); - } - - auto indexesElem = obj.getField("indexes"); - if (indexesElem.eoo()) { - return {ErrorCodes::BadValue, str::stream() << "Missing required field 'indexes'"}; - } - - if (indexesElem.type() != Array) { - return {ErrorCodes::BadValue, - str::stream() << "Field 'indexes' must be an array of index spec objects"}; - } - - std::vector<std::string> indexNames; - std::vector<BSONObj> indexSpecs; - for (auto& indexElem : indexesElem.Array()) { - if (!indexElem.isABSONObj()) { - return {ErrorCodes::BadValue, - str::stream() << "Element of 'indexes' must be an object"}; - } - std::string indexName; - auto status = bsonExtractStringField(indexElem.Obj(), "name", &indexName); - if (!status.isOK()) { - return status.withContext("Error extracting 'name' from index spec"); - } - indexNames.push_back(indexName); - indexSpecs.push_back(indexElem.Obj().getOwned()); - } - return IndexBuildOplogEntry{swBuildUUID.getValue(), indexNames, indexSpecs}; -} - -} // namespace - Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* opCtx, const OplogInterface& localOplog, FixUpInfo& fixUpInfo, @@ -507,15 +436,15 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o return Status::OK(); } case OplogEntry::CommandType::kStartIndexBuild: { - auto swIndexBuildOplogObject = parseIndexBuildOplogObject(obj); - if (!swIndexBuildOplogObject.isOK()) { + auto swIndexBuildOplogEntry = IndexBuildOplogEntry::parse(oplogEntry); + if (!swIndexBuildOplogEntry.isOK()) { return {ErrorCodes::UnrecoverableRollbackError, str::stream() << "Error parsing 'startIndexBuild' oplog entry: " - << swIndexBuildOplogObject.getStatus() << ": " << redact(obj)}; + << swIndexBuildOplogEntry.getStatus() << ": " << redact(obj)}; } - auto& indexBuildOplogObject = swIndexBuildOplogObject.getValue(); + auto& indexBuildOplogEntry = swIndexBuildOplogEntry.getValue(); // If the index build has been committed or aborted, and the commit or abort // oplog entry has also been rolled back, the index build will have been added @@ -523,7 +452,7 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o // dropped. If the index has already been dropped by abort, then this is a // no-op. auto& buildsToRestart = fixUpInfo.indexBuildsToRestart; - auto buildUUID = indexBuildOplogObject.buildUUID; + auto buildUUID = indexBuildOplogEntry.buildUUID; auto existingIt = buildsToRestart.find(buildUUID); if (existingIt != buildsToRestart.end()) { LOG(2) << "Index build that was previously marked to be restarted will now be " @@ -533,7 +462,7 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o // If the index build was committed or aborted, we must mark the index as // needing to be dropped. Add each index to drop by name individually. - for (auto& indexName : indexBuildOplogObject.indexNames) { + for (auto& indexName : indexBuildOplogEntry.indexNames) { fixUpInfo.indexesToDrop[*uuid].insert(indexName); } return Status::OK(); @@ -542,24 +471,24 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o // If the index build was not committed or aborted, the index build is // unfinished in the catalog will need to be dropped before any other collection // operations. - for (auto& indexName : indexBuildOplogObject.indexNames) { + for (auto& indexName : indexBuildOplogEntry.indexNames) { fixUpInfo.unfinishedIndexesToDrop[*uuid].insert(indexName); } return Status::OK(); } case OplogEntry::CommandType::kAbortIndexBuild: { - auto swIndexBuildOplogObject = parseIndexBuildOplogObject(obj); - if (!swIndexBuildOplogObject.isOK()) { + auto swIndexBuildOplogEntry = IndexBuildOplogEntry::parse(oplogEntry); + if (!swIndexBuildOplogEntry.isOK()) { return {ErrorCodes::UnrecoverableRollbackError, str::stream() << "Error parsing 'abortIndexBuild' oplog entry: " - << swIndexBuildOplogObject.getStatus() << ": " << redact(obj)}; + << swIndexBuildOplogEntry.getStatus() << ": " << redact(obj)}; } - auto& indexBuildOplogObject = swIndexBuildOplogObject.getValue(); + auto& indexBuildOplogEntry = swIndexBuildOplogEntry.getValue(); auto& buildsToRestart = fixUpInfo.indexBuildsToRestart; - auto buildUUID = indexBuildOplogObject.buildUUID; + auto buildUUID = indexBuildOplogEntry.buildUUID; invariant(buildsToRestart.find(buildUUID) == buildsToRestart.end(), str::stream() << "Tried to restart an index build after rolling back an " @@ -570,7 +499,7 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o LOG(2) << "Index build will be restarted after a rolled-back 'abortIndexBuild': " << buildUUID; IndexBuildDetails details{*uuid}; - for (auto& spec : indexBuildOplogObject.indexSpecs) { + for (auto& spec : indexBuildOplogEntry.indexSpecs) { invariant(spec.isOwned()); details.indexSpecs.emplace_back(spec); } @@ -578,21 +507,21 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o return Status::OK(); } case OplogEntry::CommandType::kCommitIndexBuild: { - auto swIndexBuildOplogObject = parseIndexBuildOplogObject(obj); - if (!swIndexBuildOplogObject.isOK()) { + auto swIndexBuildOplogEntry = IndexBuildOplogEntry::parse(oplogEntry); + if (!swIndexBuildOplogEntry.isOK()) { return {ErrorCodes::UnrecoverableRollbackError, str::stream() << "Error parsing 'commitIndexBuild' oplog entry: " - << swIndexBuildOplogObject.getStatus() << ": " << redact(obj)}; + << swIndexBuildOplogEntry.getStatus() << ": " << redact(obj)}; } - auto& indexBuildOplogObject = swIndexBuildOplogObject.getValue(); + auto& indexBuildOplogEntry = swIndexBuildOplogEntry.getValue(); // If a dropIndexes oplog entry was already rolled-back, the index build needs to // be restarted, but not committed. If the index is in the set to be created, then // its drop was rolled-back and it should be removed. auto& toCreate = fixUpInfo.indexesToCreate[*uuid]; - for (auto& indexName : indexBuildOplogObject.indexNames) { + for (auto& indexName : indexBuildOplogEntry.indexNames) { auto existing = toCreate.find(indexName); if (existing != toCreate.end()) { toCreate.erase(existing); @@ -601,7 +530,7 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o // Add the index build to be restarted. auto& buildsToRestart = fixUpInfo.indexBuildsToRestart; - auto buildUUID = indexBuildOplogObject.buildUUID; + auto buildUUID = indexBuildOplogEntry.buildUUID; invariant(buildsToRestart.find(buildUUID) == buildsToRestart.end(), str::stream() << "Tried to restart an index build after rolling back a " @@ -613,7 +542,7 @@ Status rollback_internal::updateFixUpInfoFromLocalOplogEntry(OperationContext* o << buildUUID; IndexBuildDetails details{*uuid}; - for (auto& spec : indexBuildOplogObject.indexSpecs) { + for (auto& spec : indexBuildOplogEntry.indexSpecs) { invariant(spec.isOwned()); details.indexSpecs.emplace_back(spec); } diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp index 29c366390c9..462ca0f708c 100644 --- a/src/mongo/db/repl/rs_rollback_test.cpp +++ b/src/mongo/db/repl/rs_rollback_test.cpp @@ -132,8 +132,14 @@ OplogInterfaceMock::Operation makeAbortIndexBuildOplogEntry(Collection* collecti UUID buildUUID, BSONObj spec, int time) { - auto entry = BSON("abortIndexBuild" << collection->ns().coll() << "indexBuildUUID" << buildUUID - << "indexes" << BSON_ARRAY(spec)); + Status cause = {ErrorCodes::IndexBuildAborted, "test"}; + + BSONObjBuilder causeBuilder; + causeBuilder.appendBool("ok", 0); + cause.serializeErrorToBSON(&causeBuilder); + auto entry = + BSON("abortIndexBuild" << collection->ns().coll() << "indexBuildUUID" << buildUUID + << "indexes" << BSON_ARRAY(spec) << "cause" << causeBuilder.done()); return std::make_pair(BSON("ts" << Timestamp(Seconds(time), 0) << "op" << "c" |