From 281e5d5cf9742c74448f0c8600912c6f6ca8326d Mon Sep 17 00:00:00 2001
From: Louis Williams <louis.williams@mongodb.com>
Date: Tue, 15 Jan 2019 11:25:41 -0500
Subject: SERVER-38748 Background indexes created through applyOps should run
 on the command thread

(cherry picked from commit 9c49d721526ac83ada34950841ceef5b0b48c3c5)
---
 jstests/core/apply_ops_invalid_index_spec.js | 18 ++++++++++
 src/mongo/db/repl/oplog.cpp                  | 53 ++++++++++++++++++----------
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/jstests/core/apply_ops_invalid_index_spec.js b/jstests/core/apply_ops_invalid_index_spec.js
index f08359c2403..f01cfc8cbc3 100644
--- a/jstests/core/apply_ops_invalid_index_spec.js
+++ b/jstests/core/apply_ops_invalid_index_spec.js
@@ -50,6 +50,24 @@
     }),
                                  ErrorCodes.InvalidIndexSpecificationOption);
 
+    // A createIndexes command for a background index with unknown field in the index spec should
+    // fail.
+    assert.commandFailedWithCode(db.adminCommand({
+        applyOps: [{
+            op: 'c',
+            ns: cmdNs,
+            o: {
+                createIndexes: t.getName(),
+                v: 2,
+                key: {a: 1},
+                background: true,
+                name: 'a_1_background',
+                unknown: 1,
+            },
+        }],
+    }),
+                                 ErrorCodes.InvalidIndexSpecificationOption);
+
     // A createIndexes command for a v:1 index with an unknown field in the index spec should work.
     const res1 = assert.commandWorked(db.adminCommand({
         applyOps: [{
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 3c99231a7ef..5d49daa71ee 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -222,6 +222,26 @@ private:
     BSONObj _oField;
 };
 
+bool shouldBuildInForeground(OperationContext* opCtx,
+                             const BSONObj& index,
+                             const NamespaceString& indexNss) {
+    if (!index["background"].trueValue()) {
+        return true;
+    }
+
+    // Primaries should build indexes in the foreground because failures cannot be handled
+    // by the background thread.
+    const bool isPrimary =
+        repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(opCtx, indexNss);
+    if (isPrimary) {
+        LOG(3) << "apply op: not building background index " << index
+               << " in a background thread because this is a primary";
+        return true;
+    }
+    return false;
+}
+
+
 }  // namespace
 
 void setOplogCollectionName() {
@@ -266,28 +286,25 @@ 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.
-            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);
-            // This spawns a new thread and returns immediately.
-            builder->go();
-            // Wait for thread to start and register itself
-            IndexBuilder::waitForBgIndexStarting();
-        }
-        opCtx->recoveryUnit()->abandonSnapshot();
-    } else {
+
+    if (shouldBuildInForeground(opCtx, indexSpec, indexNss)) {
         IndexBuilder builder(indexSpec, relaxIndexConstraints);
         Status status = builder.buildInForeground(opCtx, db);
         uassertStatusOK(status);
+    } else {
+        Lock::TempRelease release(opCtx->lockState());
+        // TempRelease cannot fail because no recursive locks should be taken.
+        invariant(!opCtx->lockState()->isLocked());
+
+        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();
     }
+
+    opCtx->recoveryUnit()->abandonSnapshot();
+
     if (incrementOpsAppliedStats) {
         incrementOpsAppliedStats();
     }
-- 
cgit v1.2.1