SERVER-57737 Index builds must save and restore the collection scanning cursor around writes to the

violated index key constraints side table in case the write throws and unpositions the read cursor.

(cherry picked from commit 05f055aad47b6afa3902ec4c11925c848f9cd534)

7 files changed, 109 insertions, 23 deletions

diff --git a/src/mongo/db/catalog/index_builds_manager.cpp b/src/mongo/db/catalog/index_builds_manager.cpp
index 8d4e3eb973a..6509bb6a67e 100644
--- a/src/mongo/db/catalog/index_builds_manager.cpp
+++ b/src/mongo/db/catalog/index_builds_manager.cpp
@@ -192,7 +192,17 @@ StatusWith<std::pair<long long, long long>> IndexBuildsManager::startBuildingInd
                     numRecords++;
                     dataSize += data.size();
                     auto insertStatus = builder->insertSingleDocumentForInitialSyncOrRecovery(
-                        opCtx, data.releaseToBson(), id);
+                        opCtx,
+                        data.releaseToBson(),
+                        id,
+                        [&cursor] { cursor->save(); },
+                        [&] {
+                            writeConflictRetry(
+                                opCtx,
+                                "insertSingleDocumentForInitialSyncOrRecovery-restoreCursor",
+                                ns.ns(),
+                                [&cursor] { cursor->restore(); });
+                        });
                     if (!insertStatus.isOK()) {
                         return insertStatus;
                     }
diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
index 71ee6ff7fd8..64cb79fa34e 100644
--- a/src/mongo/db/catalog/multi_index_block.cpp
+++ b/src/mongo/db/catalog/multi_index_block.cpp
@@ -594,7 +594,18 @@ void MultiIndexBlock::_doCollectionScan(OperationContext* opCtx,
 
         // The external sorter is not part of the storage engine and therefore does not need
         // a WriteUnitOfWork to write keys.
-        uassertStatusOK(_insert(opCtx, objToIndex, loc));
+        //
+        // However, if a key constraint violation is found, it will be written to the constraint
+        // violations side table. The plan executor must be passed down to save and restore the
+        // cursor around the side table write in case any write conflict exception occurs that would
+        // otherwise reposition the cursor unexpectedly. All WUOW and write conflict exception
+        // handling for the side table write is handled internally.
+        uassertStatusOK(
+            _insert(opCtx,
+                    objToIndex,
+                    loc,
+                    /*saveCursorBeforeWrite*/ [&exec] { exec->saveState(); },
+                    /*restoreCursorAfterWrite*/ [&] { exec->restoreState(&collection); }));
 
         _failPointHangDuringBuild(opCtx,
                                   &hangIndexBuildDuringCollectionScanPhaseAfterInsertion,
@@ -608,13 +619,20 @@ void MultiIndexBlock::_doCollectionScan(OperationContext* opCtx,
     }
 }
 
-Status MultiIndexBlock::insertSingleDocumentForInitialSyncOrRecovery(OperationContext* opCtx,
-                                                                     const BSONObj& doc,
-                                                                     const RecordId& loc) {
-    return _insert(opCtx, doc, loc);
+Status MultiIndexBlock::insertSingleDocumentForInitialSyncOrRecovery(
+    OperationContext* opCtx,
+    const BSONObj& doc,
+    const RecordId& loc,
+    const std::function<void()>& saveCursorBeforeWrite,
+    const std::function<void()>& restoreCursorAfterWrite) {
+    return _insert(opCtx, doc, loc, saveCursorBeforeWrite, restoreCursorAfterWrite);
 }
 
-Status MultiIndexBlock::_insert(OperationContext* opCtx, const BSONObj& doc, const RecordId& loc) {
+Status MultiIndexBlock::_insert(OperationContext* opCtx,
+                                const BSONObj& doc,
+                                const RecordId& loc,
+                                const std::function<void()>& saveCursorBeforeWrite,
+                                const std::function<void()>& restoreCursorAfterWrite) {
     invariant(!_buildIsCleanedUp);
     for (size_t i = 0; i < _indexes.size(); i++) {
         if (_indexes[i].filterExpression && !_indexes[i].filterExpression->matchesBSON(doc)) {
@@ -626,7 +644,12 @@ Status MultiIndexBlock::_insert(OperationContext* opCtx, const BSONObj& doc, con
         // When calling insert, BulkBuilderImpl's Sorter performs file I/O that may result in an
         // exception.
         try {
-            idxStatus = _indexes[i].bulk->insert(opCtx, doc, loc, _indexes[i].options);
+            idxStatus = _indexes[i].bulk->insert(opCtx,
+                                                 doc,
+                                                 loc,
+                                                 _indexes[i].options,
+                                                 saveCursorBeforeWrite,
+                                                 restoreCursorAfterWrite);
         } catch (...) {
             return exceptionToStatus();
         }
diff --git a/src/mongo/db/catalog/multi_index_block.h b/src/mongo/db/catalog/multi_index_block.h
index 58342374e6b..12b655d73d0 100644
--- a/src/mongo/db/catalog/multi_index_block.h
+++ b/src/mongo/db/catalog/multi_index_block.h
@@ -164,10 +164,19 @@ public:
      * Do not call if you called insertAllDocumentsInCollection();
      *
      * Should be called inside of a WriteUnitOfWork.
+     *
+     * 'saveCursorBeforeWrite' and 'restoreCursorAfterWrite' will only be called if an index
+     * constraint violation is found and written out to the constraint violation side table. Any
+     * open cursors held by the caller should be saved in 'saveCursorBeforeWrite' and restored in
+     * 'saveCursorBeforeWrite'. Otherwise, the cursors may get reset unexpectedly because of an
+     * internally handled WCE.
      */
-    Status insertSingleDocumentForInitialSyncOrRecovery(OperationContext* opCtx,
-                                                        const BSONObj& wholeDocument,
-                                                        const RecordId& loc);
+    Status insertSingleDocumentForInitialSyncOrRecovery(
+        OperationContext* opCtx,
+        const BSONObj& wholeDocument,
+        const RecordId& loc,
+        const std::function<void()>& saveCursorBeforeWrite,
+        const std::function<void()>& restoreCursorAfterWrite);
 
     /**
      * Call this after the last insertSingleDocumentForInitialSyncOrRecovery(). This gives the index
@@ -311,7 +320,11 @@ private:
                                      const BSONObj& doc,
                                      unsigned long long iteration) const;
 
-    Status _insert(OperationContext* opCtx, const BSONObj& wholeDocument, const RecordId& loc);
+    Status _insert(OperationContext* opCtx,
+                   const BSONObj& wholeDocument,
+                   const RecordId& loc,
+                   const std::function<void()>& saveCursorBeforeWrite,
+                   const std::function<void()>& restoreCursorAfterWrite);
 
     /**
      * Performs a collection scan on the given collection and inserts the relevant index keys into
diff --git a/src/mongo/db/catalog/multi_index_block_test.cpp b/src/mongo/db/catalog/multi_index_block_test.cpp
index 6edbb173115..dc09f95d63b 100644
--- a/src/mongo/db/catalog/multi_index_block_test.cpp
+++ b/src/mongo/db/catalog/multi_index_block_test.cpp
@@ -120,7 +120,12 @@ TEST_F(MultiIndexBlockTest, CommitAfterInsertingSingleDocument) {
         operationContext(), coll, std::vector<BSONObj>(), MultiIndexBlock::kNoopOnInitFn));
     ASSERT_EQUALS(0U, specs.size());
 
-    ASSERT_OK(indexer->insertSingleDocumentForInitialSyncOrRecovery(operationContext(), {}, {}));
+    ASSERT_OK(
+        indexer->insertSingleDocumentForInitialSyncOrRecovery(operationContext(),
+                                                              {},
+                                                              {},
+                                                              /*saveCursorBeforeWrite*/ []() {},
+                                                              /*restoreCursorAfterWrite*/ []() {}));
     ASSERT_OK(indexer->dumpInsertsFromBulk(operationContext(), coll.get()));
     ASSERT_OK(indexer->checkConstraints(operationContext(), coll.get()));
 
@@ -146,7 +151,12 @@ TEST_F(MultiIndexBlockTest, AbortWithoutCleanupAfterInsertingSingleDocument) {
     auto specs = unittest::assertGet(indexer->init(
         operationContext(), coll, std::vector<BSONObj>(), MultiIndexBlock::kNoopOnInitFn));
     ASSERT_EQUALS(0U, specs.size());
-    ASSERT_OK(indexer->insertSingleDocumentForInitialSyncOrRecovery(operationContext(), {}, {}));
+    ASSERT_OK(
+        indexer->insertSingleDocumentForInitialSyncOrRecovery(operationContext(),
+                                                              {},
+                                                              {},
+                                                              /*saveCursorBeforeWrite*/ []() {},
+                                                              /*restoreCursorAfterWrite*/ []() {}));
     auto isResumable = false;
     indexer->abortWithoutCleanup(operationContext(), coll.get(), isResumable);
 }
diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp
index 9d679e1459e..c57d8969a91 100644
--- a/src/mongo/db/index/index_access_method.cpp
+++ b/src/mongo/db/index/index_access_method.cpp
@@ -487,7 +487,9 @@ public:
     Status insert(OperationContext* opCtx,
                   const BSONObj& obj,
                   const RecordId& loc,
-                  const InsertDeleteOptions& options) final;
+                  const InsertDeleteOptions& options,
+                  const std::function<void()>& saveCursorBeforeWrite,
+                  const std::function<void()>& restoreCursorAfterWrite) final;
 
     const MultikeyPaths& getMultikeyPaths() const final;
 
@@ -557,10 +559,13 @@ AbstractIndexAccessMethod::BulkBuilderImpl::BulkBuilderImpl(const IndexCatalogEn
       _isMultiKey(stateInfo.getIsMultikey()),
       _indexMultikeyPaths(createMultikeyPaths(stateInfo.getMultikeyPaths())) {}
 
-Status AbstractIndexAccessMethod::BulkBuilderImpl::insert(OperationContext* opCtx,
-                                                          const BSONObj& obj,
-                                                          const RecordId& loc,
-                                                          const InsertDeleteOptions& options) {
+Status AbstractIndexAccessMethod::BulkBuilderImpl::insert(
+    OperationContext* opCtx,
+    const BSONObj& obj,
+    const RecordId& loc,
+    const InsertDeleteOptions& options,
+    const std::function<void()>& saveCursorBeforeWrite,
+    const std::function<void()>& restoreCursorAfterWrite) {
     auto& executionCtx = StorageExecutionContext::get(opCtx);
 
     auto keys = executionCtx.keys();
@@ -588,7 +593,12 @@ Status AbstractIndexAccessMethod::BulkBuilderImpl::insert(OperationContext* opCt
                                 "error"_attr = status,
                                 "loc"_attr = loc,
                                 "obj"_attr = redact(obj));
+
+                    // Save and restore the cursor around the write in case it throws a WCE
+                    // internally and causes the cursor to be unpositioned.
+                    saveCursorBeforeWrite();
                     interceptor->getSkippedRecordTracker()->record(opCtx, loc);
+                    restoreCursorAfterWrite();
                 }
             });
     } catch (...) {
diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h
index 417af5798b0..136e845c1ec 100644
--- a/src/mongo/db/index/index_access_method.h
+++ b/src/mongo/db/index/index_access_method.h
@@ -238,11 +238,21 @@ public:
 
         /**
          * Insert into the BulkBuilder as-if inserting into an IndexAccessMethod.
+         *
+         * 'saveCursorBeforeWrite' and 'restoreCursorAfterWrite' will be used to save and restore
+         * the cursor around any constraint violation side table write that may occur, in case a WCE
+         * occurs internally that would otherwise unposition the cursor.
+         *
+         * Note: we pass the cursor down into this insert function so we can limit cursor
+         * save/restore to around constraints violation side table writes only. Otherwise, we would
+         * have to save/restore around each insert() call just in case there is a side table write.
          */
         virtual Status insert(OperationContext* opCtx,
                               const BSONObj& obj,
                               const RecordId& loc,
-                              const InsertDeleteOptions& options) = 0;
+                              const InsertDeleteOptions& options,
+                              const std::function<void()>& saveCursorBeforeWrite,
+                              const std::function<void()>& restoreCursorAfterWrite) = 0;
 
         virtual const MultikeyPaths& getMultikeyPaths() const = 0;
 
diff --git a/src/mongo/db/repl/collection_bulk_loader_impl.cpp b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
index ed2c1aa4233..0ca613d5246 100644
--- a/src/mongo/db/repl/collection_bulk_loader_impl.cpp
+++ b/src/mongo/db/repl/collection_bulk_loader_impl.cpp
@@ -377,8 +377,13 @@ Status CollectionBulkLoaderImpl::_runTaskReleaseResourcesOnFailure(const F& task
 Status CollectionBulkLoaderImpl::_addDocumentToIndexBlocks(const BSONObj& doc,
                                                            const RecordId& loc) {
     if (_idIndexBlock) {
-        auto status =
-            _idIndexBlock->insertSingleDocumentForInitialSyncOrRecovery(_opCtx.get(), doc, loc);
+        auto status = _idIndexBlock->insertSingleDocumentForInitialSyncOrRecovery(
+            _opCtx.get(),
+            doc,
+            loc,
+            // This caller / code path does not have cursors to save/restore.
+            /*saveCursorBeforeWrite*/ []() {},
+            /*restoreCursorAfterWrite*/ []() {});
         if (!status.isOK()) {
             return status.withContext("failed to add document to _id index");
         }
@@ -386,7 +391,12 @@ Status CollectionBulkLoaderImpl::_addDocumentToIndexBlocks(const BSONObj& doc,
 
     if (_secondaryIndexesBlock) {
         auto status = _secondaryIndexesBlock->insertSingleDocumentForInitialSyncOrRecovery(
-            _opCtx.get(), doc, loc);
+            _opCtx.get(),
+            doc,
+            loc,
+            // This caller / code path does not have cursors to save/restore.
+            /*saveCursorBeforeWrite*/ []() {},
+            /*restoreCursorAfterWrite*/ []() {});
         if (!status.isOK()) {
             return status.withContext("failed to add document to secondary indexes");
         }