SERVER-44654 Allow unique, two-phase index builds to continue running on stepdown

7 files changed, 41 insertions, 80 deletions

diff --git a/src/mongo/db/catalog/index_builds_manager.cpp b/src/mongo/db/catalog/index_builds_manager.cpp
index fa15f48873a..a1b1ad27377 100644
--- a/src/mongo/db/catalog/index_builds_manager.cpp
+++ b/src/mongo/db/catalog/index_builds_manager.cpp
@@ -93,10 +93,13 @@ Status IndexBuildsManager::setUpIndexBuild(OperationContext* opCtx,
         builder->setTwoPhaseBuildUUID(buildUUID);
     }
 
-    // Ignore uniqueness constraint violations when relaxed (on secondaries). Secondaries can
-    // complete index builds in the middle of batches, which creates the potential for finding
-    // duplicate key violations where there otherwise would be none at consistent states.
-    if (options.indexConstraints == IndexConstraints::kRelax) {
+    // Ignore uniqueness constraint violations when relaxed, for single-phase builds on
+    // secondaries. Secondaries can complete index builds in the middle of batches, which creates
+    // the potential for finding duplicate key violations where there otherwise would be none at
+    // consistent states.
+    // Two-phase builds will defer any unique key violations until commit-time.
+    if (options.indexConstraints == IndexConstraints::kRelax &&
+        options.protocol == IndexBuildProtocol::kSinglePhase) {
         builder->ignoreUniqueConstraint();
     }
 
diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
index b6c4d6a1bdb..7c2cf1c2d9c 100644
--- a/src/mongo/db/catalog/multi_index_block.cpp
+++ b/src/mongo/db/catalog/multi_index_block.cpp
@@ -321,16 +321,14 @@ StatusWith<std::vector<BSONObj>> MultiIndexBlock::init(OperationContext* opCtx,
 
             // Allow duplicates when explicitly allowed or when using hybrid builds, which will
             // perform duplicate checking itself.
-            index.options.dupsAllowed = index.options.dupsAllowed || _ignoreUnique ||
-                index.block->getEntry()->isHybridBuilding();
-            if (_ignoreUnique) {
-                index.options.getKeysMode = IndexAccessMethod::GetKeysMode::kRelaxConstraints;
-            }
+            index.options.dupsAllowed =
+                index.options.dupsAllowed || index.block->getEntry()->isHybridBuilding();
 
             // Two-phase index builds (with a build UUID) always relax constraints and check for
             // violations at commit-time.
             if (_buildUUID) {
                 index.options.getKeysMode = IndexAccessMethod::GetKeysMode::kRelaxConstraints;
+                index.options.dupsAllowed = true;
             }
 
             index.options.fromIndexBuilder = true;
@@ -721,8 +719,17 @@ Status MultiIndexBlock::drainBackgroundWrites(
         if (!interceptor)
             continue;
 
+        // Track duplicates for later constraint checking for two-phase builds (with a buildUUID),
+        // whenever key constraints are being enforced (i.e. single-phase builds on primaries), and
+        // never when _ignoreUnique is set explicitly.
+        auto trackDups = !_ignoreUnique &&
+                (_buildUUID ||
+                 IndexAccessMethod::GetKeysMode::kEnforceConstraints ==
+                     _indexes[i].options.getKeysMode)
+            ? IndexBuildInterceptor::TrackDuplicates::kTrack
+            : IndexBuildInterceptor::TrackDuplicates::kNoTrack;
         auto status = interceptor->drainWritesIntoIndex(
-            opCtx, _indexes[i].options, readSource, drainYieldPolicy);
+            opCtx, _indexes[i].options, trackDups, readSource, drainYieldPolicy);
         if (!status.isOK()) {
             return status;
         }
diff --git a/src/mongo/db/index/duplicate_key_tracker.cpp b/src/mongo/db/index/duplicate_key_tracker.cpp
index 97d6ae3ab25..b334ec5c481 100644
--- a/src/mongo/db/index/duplicate_key_tracker.cpp
+++ b/src/mongo/db/index/duplicate_key_tracker.cpp
@@ -145,11 +145,9 @@ Status DuplicateKeyTracker::checkConstraints(OperationContext* opCtx) const {
     int logLevel = (resolved > 0) ? 0 : 1;
     LOGV2_DEBUG(20677,
                 logSeverityV1toV2(logLevel).toInt(),
-                "index build: resolved {resolved} duplicate key conflicts for unique index: "
-                "{indexCatalogEntry_descriptor_indexName}",
-                "resolved"_attr = resolved,
-                "indexCatalogEntry_descriptor_indexName"_attr =
-                    _indexCatalogEntry->descriptor()->indexName());
+                "index build: resolved duplicate key conflicts for unique index",
+                "numResolved"_attr = resolved,
+                "indexName"_attr = _indexCatalogEntry->descriptor()->indexName());
     return Status::OK();
 }
 
diff --git a/src/mongo/db/index/duplicate_key_tracker.h b/src/mongo/db/index/duplicate_key_tracker.h
index c5399620984..391a7d673a7 100644
--- a/src/mongo/db/index/duplicate_key_tracker.h
+++ b/src/mongo/db/index/duplicate_key_tracker.h
@@ -82,10 +82,6 @@ public:
      */
     bool areAllConstraintsChecked(OperationContext* opCtx) const;
 
-    const std::string& getConstraintsTableIdent() const {
-        return _keyConstraintsTable->rs()->getIdent();
-    }
-
 private:
     const IndexCatalogEntry* _indexCatalogEntry;
 
diff --git a/src/mongo/db/index/index_build_interceptor.cpp b/src/mongo/db/index/index_build_interceptor.cpp
index dd8c6ac852b..c1b53479b50 100644
--- a/src/mongo/db/index/index_build_interceptor.cpp
+++ b/src/mongo/db/index/index_build_interceptor.cpp
@@ -123,17 +123,9 @@ bool IndexBuildInterceptor::areAllConstraintsChecked(OperationContext* opCtx) co
     return _duplicateKeyTracker->areAllConstraintsChecked(opCtx);
 }
 
-const std::string& IndexBuildInterceptor::getSideWritesTableIdent() const {
-    return _sideWritesTable->rs()->getIdent();
-}
-
-const std::string& IndexBuildInterceptor::getConstraintViolationsTableIdent() const {
-    return _duplicateKeyTracker->getConstraintsTableIdent();
-}
-
-
 Status IndexBuildInterceptor::drainWritesIntoIndex(OperationContext* opCtx,
                                                    const InsertDeleteOptions& options,
+                                                   TrackDuplicates trackDuplicates,
                                                    RecoveryUnit::ReadSource readSource,
                                                    DrainYieldPolicy drainYieldPolicy) {
     invariant(!opCtx->lockState()->inAWriteUnitOfWork());
@@ -213,8 +205,8 @@ Status IndexBuildInterceptor::drainWritesIntoIndex(OperationContext* opCtx,
             batchSize += 1;
             batchSizeBytes += objSize;
 
-            if (auto status =
-                    _applyWrite(opCtx, unownedDoc, options, &totalInserted, &totalDeleted);
+            if (auto status = _applyWrite(
+                    opCtx, unownedDoc, options, trackDuplicates, &totalInserted, &totalDeleted);
                 !status.isOK()) {
                 return status;
             }
@@ -270,15 +262,12 @@ Status IndexBuildInterceptor::drainWritesIntoIndex(OperationContext* opCtx,
     int logLevel = (_numApplied - appliedAtStart > 0) ? 0 : 1;
     LOGV2_DEBUG(20689,
                 logSeverityV1toV2(logLevel).toInt(),
-                "index build: drain applied {numApplied_appliedAtStart} side writes (inserted: "
-                "{totalInserted}, deleted: {totalDeleted}) for "
-                "'{indexCatalogEntry_descriptor_indexName}' in {timer_millis} ms",
-                "numApplied_appliedAtStart"_attr = (_numApplied - appliedAtStart),
+                "index build: drained side writes",
+                "numApplied"_attr = (_numApplied - appliedAtStart),
                 "totalInserted"_attr = totalInserted,
                 "totalDeleted"_attr = totalDeleted,
-                "indexCatalogEntry_descriptor_indexName"_attr =
-                    _indexCatalogEntry->descriptor()->indexName(),
-                "timer_millis"_attr = timer.millis());
+                "indexName"_attr = _indexCatalogEntry->descriptor()->indexName(),
+                "durationMillis"_attr = timer.millis());
 
     return Status::OK();
 }
@@ -286,6 +275,7 @@ Status IndexBuildInterceptor::drainWritesIntoIndex(OperationContext* opCtx,
 Status IndexBuildInterceptor::_applyWrite(OperationContext* opCtx,
                                           const BSONObj& operation,
                                           const InsertDeleteOptions& options,
+                                          TrackDuplicates trackDups,
                                           int64_t* const keysInserted,
                                           int64_t* const keysDeleted) {
     // Deserialize the encoded KeyString::Value.
@@ -317,8 +307,7 @@ Status IndexBuildInterceptor::_applyWrite(OperationContext* opCtx,
             return status;
         }
 
-        if (result.dupsInserted.size() &&
-            options.getKeysMode == IndexAccessMethod::GetKeysMode::kEnforceConstraints) {
+        if (result.dupsInserted.size() && TrackDuplicates::kTrack == trackDups) {
             status = recordDuplicateKeys(opCtx, result.dupsInserted);
             if (!status.isOK()) {
                 return status;
diff --git a/src/mongo/db/index/index_build_interceptor.h b/src/mongo/db/index/index_build_interceptor.h
index d4081c3be92..ded0aaa1e33 100644
--- a/src/mongo/db/index/index_build_interceptor.h
+++ b/src/mongo/db/index/index_build_interceptor.h
@@ -60,6 +60,13 @@ public:
      */
     enum class TrackSkippedRecords { kNoTrack, kTrack };
 
+    /**
+     * Indicates whether to record duplicate keys that have been inserted into the index. When set
+     * to 'kNoTrack', inserted duplicate keys will be ignored. When set to 'kTrack', a subsequent
+     * call to checkDuplicateKeyConstraints is required.
+     */
+    enum class TrackDuplicates { kNoTrack, kTrack };
+
     static bool typeCanFastpathMultikeyUpdates(IndexType type);
 
     /**
@@ -126,6 +133,7 @@ public:
      */
     Status drainWritesIntoIndex(OperationContext* opCtx,
                                 const InsertDeleteOptions& options,
+                                TrackDuplicates trackDups,
                                 RecoveryUnit::ReadSource readSource,
                                 DrainYieldPolicy drainYieldPolicy);
 
@@ -162,16 +170,13 @@ public:
      */
     boost::optional<MultikeyPaths> getMultikeyPaths() const;
 
-    const std::string& getSideWritesTableIdent() const;
-
-    const std::string& getConstraintViolationsTableIdent() const;
-
 private:
     using SideWriteRecord = std::pair<RecordId, BSONObj>;
 
     Status _applyWrite(OperationContext* opCtx,
                        const BSONObj& doc,
                        const InsertDeleteOptions& options,
+                       TrackDuplicates trackDups,
                        int64_t* const keysInserted,
                        int64_t* const keysDeleted);
 
diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp
index 2c3acfaa118..e156245dfdf 100644
--- a/src/mongo/db/index_builds_coordinator.cpp
+++ b/src/mongo/db/index_builds_coordinator.cpp
@@ -958,23 +958,6 @@ bool IndexBuildsCoordinator::abortIndexBuildByBuildUUIDNoWait(
     return true;
 }
 
-/**
- * Returns true if index specs include any unique indexes. Due to uniqueness constraints set up at
- * the start of the index build, we are not able to support failing over a two phase index build on
- * a unique index to a new primary on stepdown.
- */
-namespace {
-// TODO(SERVER-44654): remove when unique indexes support failover
-bool containsUniqueIndexes(const std::vector<BSONObj>& specs) {
-    for (const auto& spec : specs) {
-        if (spec["unique"].trueValue()) {
-            return true;
-        }
-    }
-    return false;
-}
-}  // namespace
-
 std::size_t IndexBuildsCoordinator::getActiveIndexBuildCount(OperationContext* opCtx) {
     auto indexBuilds = _getIndexBuilds();
     // We use forEachIndexBuild() to log basic details on the current index builds and don't intend
@@ -998,26 +981,6 @@ void IndexBuildsCoordinator::onStepUp(OperationContext* opCtx) {
             return;
         }
 
-        // TODO(SERVER-44654): re-enable failover support for unique indexes.
-        if (containsUniqueIndexes(replState->indexSpecs)) {
-            // We abort unique index builds on step-up on the new primary, as opposed to on
-            // step-down on the old primary. This is because the old primary cannot generate any new
-            // 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.
-            // Use a null timestamp because the primary will generate its own timestamp with an
-            // oplog entry.
-            // Do not wait for the index build to exit, because it may reacquire locks that are not
-            // available until stepUp completes.
-            std::string abortReason("unique indexes do not support failover");
-            abortIndexBuildByBuildUUIDNoWait(opCtx,
-                                             replState->buildUUID,
-                                             IndexBuildAction::kPrimaryAbort,
-                                             boost::none,
-                                             abortReason);
-            return;
-        }
-
         {
             stdx::unique_lock<Latch> lk(replState->mutex);
             // After Sending the abort this might have stepped down and stepped back up.