SERVER-41140 All usages of MultiIndexBlock should ensure callers check for duplicate key constraints

(cherry picked from commit 0d462c7be0462ad27f68c056145de712bee65684)

12 files changed, 43 insertions, 5 deletions

diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
index 16bd21c3e98..d5b13448d31 100644
--- a/src/mongo/db/catalog/multi_index_block.cpp
+++ b/src/mongo/db/catalog/multi_index_block.cpp
@@ -758,6 +758,8 @@ Status MultiIndexBlock::drainBackgroundWrites(OperationContext* opCtx,
 }
 
 Status MultiIndexBlock::checkConstraints(OperationContext* opCtx) {
+    _constraintsChecked = true;
+
     if (State::kAborted == _getState()) {
         return {ErrorCodes::IndexBuildAborted,
                 str::stream() << "Index build aborted: " << _abortReason
@@ -821,6 +823,9 @@ 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 7b1b1d3e86c..7d5279280cd 100644
--- a/src/mongo/db/catalog/multi_index_block.h
+++ b/src/mongo/db/catalog/multi_index_block.h
@@ -339,6 +339,9 @@ private:
     // incorrect state set anywhere.
     bool _buildIsCleanedUp = true;
 
+    // Duplicate key constraints should be checked at least once in the MultiIndexBlock.
+    bool _constraintsChecked = false;
+
     // Protects member variables of this class declared below.
     mutable stdx::mutex _mutex;
 
diff --git a/src/mongo/db/catalog/multi_index_block_test.cpp b/src/mongo/db/catalog/multi_index_block_test.cpp
index 39adbc0728d..f87592cf418 100644
--- a/src/mongo/db/catalog/multi_index_block_test.cpp
+++ b/src/mongo/db/catalog/multi_index_block_test.cpp
@@ -117,6 +117,7 @@ TEST_F(MultiIndexBlockTest, CommitWithoutInsertingDocuments) {
     ASSERT_EQUALS(MultiIndexBlock::State::kRunning, indexer->getState_forTest());
 
     ASSERT_OK(indexer->dumpInsertsFromBulk(getOpCtx()));
+    ASSERT_OK(indexer->checkConstraints(getOpCtx()));
 
     ASSERT_FALSE(indexer->isCommitted());
     {
@@ -142,6 +143,7 @@ TEST_F(MultiIndexBlockTest, CommitAfterInsertingSingleDocument) {
 
     ASSERT_OK(indexer->insert(getOpCtx(), {}, {}));
     ASSERT_OK(indexer->dumpInsertsFromBulk(getOpCtx()));
+    ASSERT_OK(indexer->checkConstraints(getOpCtx()));
 
     ASSERT_FALSE(indexer->isCommitted());
     {
diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp
index f79fa7067c3..c2f40073f1e 100644
--- a/src/mongo/db/cloner.cpp
+++ b/src/mongo/db/cloner.cpp
@@ -403,6 +403,7 @@ void Cloner::copyIndexes(OperationContext* opCtx,
     auto indexInfoObjs = uassertStatusOK(
         indexer.init(opCtx, collection, prunedIndexesToBuild, MultiIndexBlock::kNoopOnInitFn));
     uassertStatusOK(indexer.insertAllDocumentsInCollection(opCtx, collection));
+    uassertStatusOK(indexer.checkConstraints(opCtx));
 
     WriteUnitOfWork wunit(opCtx);
     uassertStatusOK(indexer.commit(
diff --git a/src/mongo/db/commands/drop_indexes.cpp b/src/mongo/db/commands/drop_indexes.cpp
index 34447f9b4cf..c240d0d25dd 100644
--- a/src/mongo/db/commands/drop_indexes.cpp
+++ b/src/mongo/db/commands/drop_indexes.cpp
@@ -219,8 +219,8 @@ public:
             quickExit(EXIT_ABRUPT);
         }
 
-        auto status = indexer->insertAllDocumentsInCollection(opCtx, collection);
-        uassertStatusOK(status);
+        uassertStatusOK(indexer->insertAllDocumentsInCollection(opCtx, collection));
+        uassertStatusOK(indexer->checkConstraints(opCtx));
 
         {
             WriteUnitOfWork wunit(opCtx);
diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp
index a2f25c3475c..6f5804d17c4 100644
--- a/src/mongo/db/index_builder.cpp
+++ b/src/mongo/db/index_builder.cpp
@@ -166,9 +166,14 @@ Status IndexBuilder::_build(OperationContext* opCtx,
         Lock::CollectionLock collLock(opCtx, ns, MODE_IX);
         // WriteConflict exceptions and statuses are not expected to escape this method.
         status = indexer.insertAllDocumentsInCollection(opCtx, coll);
-    }
-    if (!status.isOK()) {
-        return status;
+        if (!status.isOK()) {
+            return status;
+        }
+
+        status = indexer.checkConstraints(opCtx);
+        if (!status.isOK()) {
+            return status;
+        }
     }
 
     status = writeConflictRetry(opCtx, "Commit index build", ns.ns(), [opCtx, coll, &indexer, &ns] {
diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp
index f0d139bb1bb..1c5e75f8a31 100644
--- a/src/mongo/db/index_builds_coordinator.cpp
+++ b/src/mongo/db/index_builds_coordinator.cpp
@@ -989,6 +989,9 @@ StatusWith<std::pair<long long, long long>> IndexBuildsCoordinator::_runIndexReb
         std::tie(numRecords, dataSize) = uassertStatusOK(
             _indexBuildsManager.startBuildingIndexForRecovery(opCtx, collection->ns(), buildUUID));
 
+        uassertStatusOK(
+            _indexBuildsManager.checkIndexConstraintViolations(opCtx, replState->buildUUID));
+
         // Commit the index build.
         uassertStatusOK(_indexBuildsManager.commitIndexBuild(opCtx,
                                                              collection,
diff --git a/src/mongo/db/repair_database_and_check_version.cpp b/src/mongo/db/repair_database_and_check_version.cpp
index 3ff3ad36613..91c17c24de9 100644
--- a/src/mongo/db/repair_database_and_check_version.cpp
+++ b/src/mongo/db/repair_database_and_check_version.cpp
@@ -168,6 +168,11 @@ Status buildMissingIdIndex(OperationContext* opCtx, Collection* collection) {
         return status;
     }
 
+    status = indexer.checkConstraints(opCtx);
+    if (!status.isOK()) {
+        return status;
+    }
+
     WriteUnitOfWork wuow(opCtx);
     status = indexer.commit(
         opCtx, collection, MultiIndexBlock::kNoopOnCreateEachFn, MultiIndexBlock::kNoopOnCommitFn);
diff --git a/src/mongo/db/repl/collection_bulk_loader_impl.cpp b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
index 56322162fe1..bcdde4a7492 100644
--- a/src/mongo/db/repl/collection_bulk_loader_impl.cpp
+++ b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
@@ -192,6 +192,10 @@ Status CollectionBulkLoaderImpl::commit() {
                 return status;
             }
 
+            // This should always return Status::OK() as secondary index builds ignore duplicate key
+            // constraints causing them to not be recorded.
+            invariant(_secondaryIndexesBlock->checkConstraints(_opCtx.get()));
+
             status = writeConflictRetry(
                 _opCtx.get(), "CollectionBulkLoaderImpl::commit", _nss.ns(), [this] {
                     WriteUnitOfWork wunit(_opCtx.get());
diff --git a/src/mongo/dbtests/indexupdatetests.cpp b/src/mongo/dbtests/indexupdatetests.cpp
index b4e93a32e83..63ed34e3871 100644
--- a/src/mongo/dbtests/indexupdatetests.cpp
+++ b/src/mongo/dbtests/indexupdatetests.cpp
@@ -142,6 +142,7 @@ public:
 
         ASSERT_OK(indexer.init(&_opCtx, coll, spec, MultiIndexBlock::kNoopOnInitFn).getStatus());
         ASSERT_OK(indexer.insertAllDocumentsInCollection(&_opCtx, coll));
+        ASSERT_OK(indexer.checkConstraints(&_opCtx));
 
         WriteUnitOfWork wunit(&_opCtx);
         ASSERT_OK(indexer.commit(
@@ -294,6 +295,10 @@ Status IndexBuildBase::createIndex(const std::string& dbname, const BSONObj& ind
     if (!status.isOK()) {
         return status;
     }
+    status = indexer.checkConstraints(&_opCtx);
+    if (!status.isOK()) {
+        return status;
+    }
     WriteUnitOfWork wunit(&_opCtx);
     ASSERT_OK(indexer.commit(&_opCtx,
                              collection(),
diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp
index e20ace4c166..7c993fa41ff 100644
--- a/src/mongo/dbtests/storage_timestamp_tests.cpp
+++ b/src/mongo/dbtests/storage_timestamp_tests.cpp
@@ -269,6 +269,7 @@ public:
         }
 
         ASSERT_OK(indexer.insertAllDocumentsInCollection(_opCtx, coll));
+        ASSERT_OK(indexer.checkConstraints(_opCtx));
 
         {
             WriteUnitOfWork wuow(_opCtx);
@@ -1894,6 +1895,7 @@ public:
         // Inserting all the documents has the side-effect of setting internal state on the index
         // builder that the index is multikey.
         ASSERT_OK(indexer.insertAllDocumentsInCollection(_opCtx, autoColl.getCollection()));
+        ASSERT_OK(indexer.checkConstraints(_opCtx));
 
         {
             WriteUnitOfWork wuow(_opCtx);
@@ -2074,6 +2076,8 @@ public:
             ASSERT_TRUE(buildingIndex->indexBuildInterceptor()->areAllWritesApplied(_opCtx));
         }
 
+        ASSERT_OK(indexer.checkConstraints(_opCtx));
+
         {
             WriteUnitOfWork wuow(_opCtx);
             ASSERT_OK(indexer.commit(
diff --git a/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp b/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp
index 8bf93308555..8ace89874da 100644
--- a/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp
+++ b/src/mongo/dbtests/wildcard_multikey_persistence_test.cpp
@@ -200,6 +200,7 @@ protected:
         ASSERT_OK(
             indexer.init(opCtx(), coll, indexSpec, MultiIndexBlock::kNoopOnInitFn).getStatus());
         ASSERT_OK(indexer.insertAllDocumentsInCollection(opCtx(), coll));
+        ASSERT_OK(indexer.checkConstraints(opCtx()));
 
         WriteUnitOfWork wunit(opCtx());
         ASSERT_OK(indexer.commit(