SERVER-34142: Convert background index builds to foreground during oplog recovery.

5 files changed, 111 insertions, 88 deletions

diff --git a/src/mongo/db/index_builder.cpp b/src/mongo/db/index_builder.cpp
index 0a6249ea830..20b247e1150 100644
--- a/src/mongo/db/index_builder.cpp
+++ b/src/mongo/db/index_builder.cpp
@@ -40,6 +40,7 @@
 #include "mongo/db/concurrency/write_conflict_exception.h"
 #include "mongo/db/curop.h"
 #include "mongo/db/db_raii.h"
+#include "mongo/db/repl/timestamp_block.h"
 #include "mongo/util/assert_util.h"
 #include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
@@ -66,10 +67,11 @@ void _setBgIndexStarting() {
 }
 }  // namespace
 
-IndexBuilder::IndexBuilder(const BSONObj& index, bool relaxConstraints)
+IndexBuilder::IndexBuilder(const BSONObj& index, bool relaxConstraints, Timestamp initIndexTs)
     : BackgroundJob(true /* self-delete */),
       _index(index.getOwned()),
       _relaxConstraints(relaxConstraints),
+      _initIndexTs(initIndexTs),
       _name(str::stream() << "repl index builder " << _indexBuildCount.addAndFetch(1)) {}
 
 IndexBuilder::~IndexBuilder() {}
@@ -118,16 +120,38 @@ void IndexBuilder::waitForBgIndexStarting() {
     _bgIndexStarting = false;
 }
 
+namespace {
+/**
+ * @param status shalt not be of code `WriteConflict`.
+ */
+Status _failIndexBuild(MultiIndexBlock& indexer, Status status, bool allowBackgroundBuilding) {
+    invariant(status.code() != ErrorCodes::WriteConflict);
+
+    if (status.code() == ErrorCodes::InterruptedAtShutdown) {
+        // leave it as-if kill -9 happened. This will be handled on restart.
+        invariant(allowBackgroundBuilding);  // Foreground builds aren't interrupted.
+        indexer.abortWithoutCleanup();
+        return status;
+    }
+
+    if (allowBackgroundBuilding) {
+        error() << "Background index build failed. Status: " << redact(status);
+        fassertFailed(50763);
+    } else {
+        return status;
+    }
+}
+}  // namespace
+
 Status IndexBuilder::_build(OperationContext* opCtx,
                             Database* db,
                             bool allowBackgroundBuilding,
                             Lock::DBLock* dbLock) const {
     const NamespaceString ns(_index["ns"].String());
 
-    Collection* c = db->getCollection(opCtx, ns);
-
+    Collection* coll = db->getCollection(opCtx, ns);
     // Collections should not be implicitly created by the index builder.
-    fassert(40409, c);
+    fassert(40409, coll);
 
     {
         stdx::lock_guard<Client> lk(*opCtx->getClient());
@@ -135,79 +159,62 @@ Status IndexBuilder::_build(OperationContext* opCtx,
         CurOp::get(opCtx)->setOpDescription_inlock(_index);
     }
 
-    bool haveSetBgIndexStarting = false;
-    while (true) {
-        Status status = Status::OK();
-        try {
-            MultiIndexBlock indexer(opCtx, c);
-            indexer.allowInterruption();
-
-            if (allowBackgroundBuilding)
-                indexer.allowBackgroundBuilding();
-
-            try {
-                status = indexer.init(_index).getStatus();
-                if (status == ErrorCodes::IndexAlreadyExists ||
-                    (status == ErrorCodes::IndexOptionsConflict && _relaxConstraints)) {
-                    LOG(1) << "Ignoring indexing error: " << redact(status);
-                    if (allowBackgroundBuilding) {
-                        // Must set this in case anyone is waiting for this build.
-                        _setBgIndexStarting();
-                    }
-                    return Status::OK();
-                }
-
-                if (status.isOK()) {
-                    if (allowBackgroundBuilding) {
-                        if (!haveSetBgIndexStarting) {
-                            _setBgIndexStarting();
-                            haveSetBgIndexStarting = true;
-                        }
-                        invariant(dbLock);
-                        dbLock->relockWithMode(MODE_IX);
-                    }
-
-                    Lock::CollectionLock colLock(opCtx->lockState(), ns.ns(), MODE_IX);
-                    status = indexer.insertAllDocumentsInCollection();
-                }
-
-                if (status.isOK()) {
-                    if (allowBackgroundBuilding) {
-                        dbLock->relockWithMode(MODE_X);
-                    }
-                    WriteUnitOfWork wunit(opCtx);
-                    indexer.commit();
-                    wunit.commit();
-                }
-                if (!status.isOK()) {
-                    error() << "bad status from index build: " << redact(status);
-                }
-            } catch (const DBException& e) {
-                status = e.toStatus();
-            }
-
-            if (allowBackgroundBuilding) {
-                dbLock->relockWithMode(MODE_X);
-                Database* reloadDb = dbHolder().get(opCtx, ns.db());
-                fassert(28553, reloadDb);
-                fassert(28554, reloadDb->getCollection(opCtx, ns));
-            }
-
-            if (status.code() == ErrorCodes::InterruptedAtShutdown) {
-                // leave it as-if kill -9 happened. This will be handled on restart.
-                invariant(allowBackgroundBuilding);  // Foreground builds aren't interrupted.
-                indexer.abortWithoutCleanup();
-            }
-        } catch (const WriteConflictException& wce) {
-            status = wce.toStatus();
+    MultiIndexBlock indexer(opCtx, coll);
+    indexer.allowInterruption();
+    if (allowBackgroundBuilding)
+        indexer.allowBackgroundBuilding();
+
+    Status status = Status::OK();
+    {
+        TimestampBlock tsBlock(opCtx, _initIndexTs);
+        status = writeConflictRetry(
+            opCtx, "Init index build", ns.ns(), [&] { return indexer.init(_index).getStatus(); });
+    }
+
+    if (status == ErrorCodes::IndexAlreadyExists ||
+        (status == ErrorCodes::IndexOptionsConflict && _relaxConstraints)) {
+        LOG(1) << "Ignoring indexing error: " << redact(status);
+        if (allowBackgroundBuilding) {
+            // Must set this in case anyone is waiting for this build.
+            _setBgIndexStarting();
         }
+        return Status::OK();
+    }
+    if (!status.isOK()) {
+        return _failIndexBuild(indexer, status, allowBackgroundBuilding);
+    }
 
-        if (status.code() != ErrorCodes::WriteConflict)
-            return status;
+    if (allowBackgroundBuilding) {
+        _setBgIndexStarting();
+        invariant(dbLock);
+        dbLock->relockWithMode(MODE_IX);
+    }
 
+    {
+        Lock::CollectionLock collLock(opCtx->lockState(), ns.ns(), MODE_IX);
+        // WriteConflict exceptions and statuses are not expected to escape this method.
+        status = indexer.insertAllDocumentsInCollection();
+    }
+    if (!status.isOK()) {
+        return _failIndexBuild(indexer, status, allowBackgroundBuilding);
+    }
 
-        LOG(2) << "WriteConflictException while creating index in IndexBuilder, retrying.";
-        opCtx->recoveryUnit()->abandonSnapshot();
+    if (allowBackgroundBuilding) {
+        dbLock->relockWithMode(MODE_X);
+    }
+    writeConflictRetry(opCtx, "Commit index build", ns.ns(), [opCtx, &indexer] {
+        WriteUnitOfWork wunit(opCtx);
+        indexer.commit();
+        wunit.commit();
+    });
+
+    if (allowBackgroundBuilding) {
+        dbLock->relockWithMode(MODE_X);
+        Database* reloadDb = dbHolder().get(opCtx, ns.db());
+        fassert(28553, reloadDb);
+        fassert(28554, reloadDb->getCollection(opCtx, ns));
     }
+
+    return Status::OK();
 }
 }
diff --git a/src/mongo/db/index_builder.h b/src/mongo/db/index_builder.h
index 02c11695eb8..1499b8cb816 100644
--- a/src/mongo/db/index_builder.h
+++ b/src/mongo/db/index_builder.h
@@ -62,10 +62,13 @@ class OperationContext;
  * like inserts on system.indexes.
  * The argument "relaxConstraints" specifies whether we should honor or ignore index constraints,
  * The ignoring of constraints is for replication due to idempotency reasons.
+ * The argument "initIndexTs" specifies the timestamp to be used to make the initial catalog write.
  */
 class IndexBuilder : public BackgroundJob {
 public:
-    IndexBuilder(const BSONObj& index, bool relaxConstraints);
+    IndexBuilder(const BSONObj& index,
+                 bool relaxConstraints,
+                 Timestamp initIndexTs = Timestamp::min());
     virtual ~IndexBuilder();
 
     virtual void run();
@@ -92,6 +95,7 @@ private:
 
     const BSONObj _index;
     const bool _relaxConstraints;
+    const Timestamp _initIndexTs;
     std::string _name;  // name of this builder, not related to the index
     static AtomicUInt32 _indexBuildCount;
 };
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index f21b2e185f1..e793ab4a6f4 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -224,7 +224,8 @@ void setOplogCollectionName(ServiceContext* service) {
 void createIndexForApplyOps(OperationContext* opCtx,
                             const BSONObj& indexSpec,
                             const NamespaceString& indexNss,
-                            IncrementOpsAppliedStatsFn incrementOpsAppliedStats) {
+                            IncrementOpsAppliedStatsFn incrementOpsAppliedStats,
+                            OplogApplication::Mode mode) {
     // Check if collection exists.
     Database* db = dbHolder().get(opCtx, indexNss.ns());
     auto indexCollection = db ? db->getCollection(opCtx, indexNss) : nullptr;
@@ -238,21 +239,31 @@ void createIndexForApplyOps(OperationContext* opCtx,
     bool relaxIndexConstraints =
         ReplicationCoordinator::get(opCtx)->shouldRelaxIndexConstraints(opCtx, indexNss);
     if (indexSpec["background"].trueValue()) {
-        Lock::TempRelease release(opCtx->lockState());
-        if (opCtx->lockState()->isLocked()) {
-            // If TempRelease fails, background index build will deadlock.
+        if (mode == OplogApplication::Mode::kRecovering) {
             LOG(3) << "apply op: building background index " << indexSpec
-                   << " in the foreground because temp release failed";
+                   << " in the foreground because the node is in recovery";
             IndexBuilder builder(indexSpec, relaxIndexConstraints);
             Status status = builder.buildInForeground(opCtx, db);
             uassertStatusOK(status);
         } else {
-            IndexBuilder* builder = new IndexBuilder(indexSpec, relaxIndexConstraints);
-            // This spawns a new thread and returns immediately.
-            builder->go();
-            // Wait for thread to start and register itself
-            IndexBuilder::waitForBgIndexStarting();
+            Lock::TempRelease release(opCtx->lockState());
+            if (opCtx->lockState()->isLocked()) {
+                // If TempRelease fails, background index build will deadlock.
+                LOG(3) << "apply op: building background index " << indexSpec
+                       << " in the foreground because temp release failed";
+                IndexBuilder builder(indexSpec, relaxIndexConstraints);
+                Status status = builder.buildInForeground(opCtx, db);
+                uassertStatusOK(status);
+            } else {
+                IndexBuilder* builder = new IndexBuilder(
+                    indexSpec, relaxIndexConstraints, opCtx->recoveryUnit()->getCommitTimestamp());
+                // This spawns a new thread and returns immediately.
+                builder->go();
+                // Wait for thread to start and register itself
+                IndexBuilder::waitForBgIndexStarting();
+            }
         }
+
         opCtx->recoveryUnit()->abandonSnapshot();
     } else {
         IndexBuilder builder(indexSpec, relaxIndexConstraints);
@@ -794,7 +805,7 @@ std::map<std::string, ApplyOpMetadata> opsMap = {
           BSONObj nsObj = BSON("ns" << nss.ns());
           indexSpec = indexSpec.addField(nsObj.firstElement());
 
-          createIndexForApplyOps(opCtx, indexSpec, nss, {});
+          createIndexForApplyOps(opCtx, indexSpec, nss, {}, mode);
           return Status::OK();
       },
       {ErrorCodes::IndexAlreadyExists, ErrorCodes::NamespaceNotFound}}},
@@ -1169,7 +1180,7 @@ Status applyOperation_inlock(OperationContext* opCtx,
             NamespaceString indexNss;
             std::tie(indexSpec, indexNss) =
                 repl::prepForApplyOpsIndexInsert(fieldO, op, requestNss);
-            createIndexForApplyOps(opCtx, indexSpec, indexNss, incrementOpsAppliedStats);
+            createIndexForApplyOps(opCtx, indexSpec, indexNss, incrementOpsAppliedStats, mode);
             return Status::OK();
         }
         uassert(ErrorCodes::NamespaceNotFound,
diff --git a/src/mongo/db/repl/oplog.h b/src/mongo/db/repl/oplog.h
index 08c82d43477..7c569e7d3aa 100644
--- a/src/mongo/db/repl/oplog.h
+++ b/src/mongo/db/repl/oplog.h
@@ -258,7 +258,8 @@ void signalOplogWaiters();
 void createIndexForApplyOps(OperationContext* opCtx,
                             const BSONObj& indexSpec,
                             const NamespaceString& indexNss,
-                            IncrementOpsAppliedStatsFn incrementOpsAppliedStats);
+                            IncrementOpsAppliedStatsFn incrementOpsAppliedStats,
+                            OplogApplication::Mode mode);
 
 /**
  * Allocates optimes for new entries in the oplog.  Returns an OplogSlot or a vector of OplogSlots,
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index 46597cf6671..e9ce7c1d684 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -728,7 +728,7 @@ void rollbackDropIndexes(OperationContext* opCtx,
         log() << "Creating index in rollback for collection: " << nss << ", UUID: " << uuid
               << ", index: " << indexName;
 
-        createIndexForApplyOps(opCtx, indexSpec, nss, {});
+        createIndexForApplyOps(opCtx, indexSpec, nss, {}, OplogApplication::Mode::kRecovering);
 
         LOG(1) << "Created index in rollback for collection: " << nss << ", UUID: " << uuid
                << ", index: " << indexName;