SERVER-46895 Do not check duplicate key violations on index builds during oplog application phase of initial sync

8 files changed, 16 insertions, 47 deletions

diff --git a/src/mongo/db/catalog/index_build_block.cpp b/src/mongo/db/catalog/index_build_block.cpp
index 32cfb4a88d8..b75c3b3e450 100644
--- a/src/mongo/db/catalog/index_build_block.cpp
+++ b/src/mongo/db/catalog/index_build_block.cpp
@@ -168,9 +168,6 @@ void IndexBuildBlock::success(OperationContext* opCtx, Collection* collection) {
 
         // An index build should never be completed with writes remaining in the interceptor.
         invariant(_indexBuildInterceptor->areAllWritesApplied(opCtx));
-
-        // An index build should never be completed without resolving all key constraints.
-        invariant(_indexBuildInterceptor->areAllConstraintsChecked(opCtx));
     }
 
     LOGV2(20345,
diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
index f96bb31d426..d1064939b34 100644
--- a/src/mongo/db/catalog/multi_index_block.cpp
+++ b/src/mongo/db/catalog/multi_index_block.cpp
@@ -751,8 +751,6 @@ Status MultiIndexBlock::retrySkippedRecords(OperationContext* opCtx, Collection*
 }
 
 Status MultiIndexBlock::checkConstraints(OperationContext* opCtx) {
-    _constraintsChecked = true;
-
     if (State::kAborted == _getState()) {
         return {ErrorCodes::IndexBuildAborted,
                 str::stream() << "Index build aborted: " << _abortReason
@@ -820,9 +818,6 @@ Status MultiIndexBlock::commit(OperationContext* opCtx,
                           << (_collectionUUID ? (" (" + _collectionUUID->toString() + ")") : "")};
     }
 
-    // Ensure that duplicate key constraints were checked at least once.
-    invariant(_constraintsChecked);
-
     // Do not interfere with writing multikey information when committing index builds.
     auto restartTracker = makeGuard(
         [this, opCtx] { MultikeyPathTracker::get(opCtx).startTrackingMultikeyPathInfo(); });
diff --git a/src/mongo/db/catalog/multi_index_block.h b/src/mongo/db/catalog/multi_index_block.h
index c6a2058589b..dba00abe71f 100644
--- a/src/mongo/db/catalog/multi_index_block.h
+++ b/src/mongo/db/catalog/multi_index_block.h
@@ -371,9 +371,6 @@ private:
     // incorrect state set anywhere.
     bool _buildIsCleanedUp = true;
 
-    // Duplicate key constraints should be checked at least once in the MultiIndexBlock.
-    bool _constraintsChecked = false;
-
     // A unique identifier associating this index build with a two-phase index build within a
     // replica set.
     boost::optional<UUID> _buildUUID;
diff --git a/src/mongo/db/index/duplicate_key_tracker.cpp b/src/mongo/db/index/duplicate_key_tracker.cpp
index b334ec5c481..5ddec2d8e74 100644
--- a/src/mongo/db/index/duplicate_key_tracker.cpp
+++ b/src/mongo/db/index/duplicate_key_tracker.cpp
@@ -151,15 +151,4 @@ Status DuplicateKeyTracker::checkConstraints(OperationContext* opCtx) const {
     return Status::OK();
 }
 
-bool DuplicateKeyTracker::areAllConstraintsChecked(OperationContext* opCtx) const {
-    auto cursor = _keyConstraintsTable->rs()->getCursor(opCtx);
-    auto record = cursor->next();
-
-    // The table is empty only when there are no more constraints to check.
-    if (!record)
-        return true;
-
-    return false;
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/index/duplicate_key_tracker.h b/src/mongo/db/index/duplicate_key_tracker.h
index 391a7d673a7..5034bca4c02 100644
--- a/src/mongo/db/index/duplicate_key_tracker.h
+++ b/src/mongo/db/index/duplicate_key_tracker.h
@@ -76,12 +76,6 @@ public:
      */
     Status checkConstraints(OperationContext* opCtx) const;
 
-    /**
-     * Returns true if all recorded duplicate key constraint violations have been deleted from the
-     * temporary record store.
-     */
-    bool areAllConstraintsChecked(OperationContext* opCtx) const;
-
 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 c1b53479b50..77d87b00a4b 100644
--- a/src/mongo/db/index/index_build_interceptor.cpp
+++ b/src/mongo/db/index/index_build_interceptor.cpp
@@ -116,13 +116,6 @@ Status IndexBuildInterceptor::checkDuplicateKeyConstraints(OperationContext* opC
     return _duplicateKeyTracker->checkConstraints(opCtx);
 }
 
-bool IndexBuildInterceptor::areAllConstraintsChecked(OperationContext* opCtx) const {
-    if (!_duplicateKeyTracker) {
-        return true;
-    }
-    return _duplicateKeyTracker->areAllConstraintsChecked(opCtx);
-}
-
 Status IndexBuildInterceptor::drainWritesIntoIndex(OperationContext* opCtx,
                                                    const InsertDeleteOptions& options,
                                                    TrackDuplicates trackDuplicates,
diff --git a/src/mongo/db/index/index_build_interceptor.h b/src/mongo/db/index/index_build_interceptor.h
index ded0aaa1e33..695cbfd3e39 100644
--- a/src/mongo/db/index/index_build_interceptor.h
+++ b/src/mongo/db/index/index_build_interceptor.h
@@ -159,12 +159,6 @@ public:
     bool areAllWritesApplied(OperationContext* opCtx) const;
 
     /**
-     * Returns true if all recorded duplicate key constraint violations have been checked.
-     */
-    bool areAllConstraintsChecked(OperationContext* opCtx) const;
-
-
-    /**
      * When an index builder wants to commit, use this to retrieve any recorded multikey paths
      * that were tracked during the build.
      */
diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp
index 9c1978ec096..3fc04b8aff5 100644
--- a/src/mongo/db/index_builds_coordinator.cpp
+++ b/src/mongo/db/index_builds_coordinator.cpp
@@ -2148,9 +2148,11 @@ void IndexBuildsCoordinator::_insertKeysFromSideTablesAndCommit(
         RecoveryUnit::ReadSource::kUnset,
         IndexBuildInterceptor::DrainYieldPolicy::kNoYield));
 
-    // Retry indexing records that may have been skipped while relaxing constraints (i.e. as
-    // secondary), but only if we are primary and committing the index build and during two-phase
-    // builds. Single-phase index builds are not resilient to state transitions.
+    // Retry indexing records that failed key generation while relaxing constraints (i.e. while
+    // a secondary node), but only if we are primary and committing the index build and during
+    // two-phase builds. Single-phase index builds are not resilient to state transitions and do not
+    // track skipped records. Secondaries rely on the primary's decision to commit as assurance that
+    // it has checked all key generation errors on its behalf.
     auto replCoord = repl::ReplicationCoordinator::get(opCtx);
     if (IndexBuildProtocol::kTwoPhase == replState->protocol &&
         replCoord->canAcceptWritesFor(opCtx, collection->ns())) {
@@ -2158,9 +2160,17 @@ void IndexBuildsCoordinator::_insertKeysFromSideTablesAndCommit(
             _indexBuildsManager.retrySkippedRecords(opCtx, replState->buildUUID, collection));
     }
 
-    // Index constraint checking phase.
-    uassertStatusOK(
-        _indexBuildsManager.checkIndexConstraintViolations(opCtx, replState->buildUUID));
+    // Duplicate key constraint checking phase. Duplicate key errors are tracked for single-phase
+    // builds on primaries and two-phase builds in all replication states. Single-phase builds on
+    // secondaries don't track duplicates so this call is a no-op. This can be called for two-phase
+    // builds in all replication states except during initial sync when this node is not guaranteed
+    // to be consistent.
+    bool twoPhaseAndNotInitialSyncing = IndexBuildProtocol::kTwoPhase == replState->protocol &&
+        !replCoord->getMemberState().startup2();
+    if (IndexBuildProtocol::kSinglePhase == replState->protocol || twoPhaseAndNotInitialSyncing) {
+        uassertStatusOK(
+            _indexBuildsManager.checkIndexConstraintViolations(opCtx, replState->buildUUID));
+    }
 
     // If two phase index builds is enabled, index build will be coordinated using
     // startIndexBuild and commitIndexBuild oplog entries.