SERVER-38054: failCommand fail point causes errors when run concurrently with other tests

4 files changed, 131 insertions, 28 deletions

diff --git a/jstests/core/failcommand_failpoint.js b/jstests/core/failcommand_failpoint.js
index c87347fa075..cfb9a64d41a 100644
--- a/jstests/core/failcommand_failpoint.js
+++ b/jstests/core/failcommand_failpoint.js
@@ -7,11 +7,24 @@
     const testDB = db.getSiblingDB("test_failcommand");
     const adminDB = db.getSiblingDB("admin");
 
+    const getThreadName = function() {
+        let myUri = adminDB.runCommand({whatsmyuri: 1}).you;
+        return adminDB.aggregate([{$currentOp: {localOps: true}}, {$match: {client: myUri}}])
+            .toArray()[0]
+            .desc;
+    };
+
+    let threadName = getThreadName();
+
     // Test failing with a particular error code.
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: "alwaysOn",
-        data: {errorCode: ErrorCodes.NotMaster, failCommands: ["find"]}
+        data: {
+            errorCode: ErrorCodes.NotMaster,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandFailedWithCode(testDB.runCommand({find: "c"}), ErrorCodes.NotMaster);
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
@@ -20,7 +33,11 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: "alwaysOn",
-        data: {errorCode: ErrorCodes.BadValue, failCommands: ["find"]}
+        data: {
+            errorCode: ErrorCodes.BadValue,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandFailedWithCode(testDB.runCommand({find: "c"}), ErrorCodes.BadValue);
     assert.commandWorked(testDB.runCommand({isMaster: 1}));
@@ -32,7 +49,11 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: "alwaysOn",
-        data: {errorCode: ErrorCodes.BadValue, failCommands: ["find", "isMaster"]}
+        data: {
+            errorCode: ErrorCodes.BadValue,
+            failCommands: ["find", "isMaster"],
+            threadName: threadName,
+        }
     }));
     assert.commandFailedWithCode(testDB.runCommand({find: "c"}), ErrorCodes.BadValue);
     assert.commandFailedWithCode(testDB.runCommand({isMaster: 1}), ErrorCodes.BadValue);
@@ -42,7 +63,11 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {skip: 2},
-        data: {errorCode: ErrorCodes.NotMaster, failCommands: ["find"]}
+        data: {
+            errorCode: ErrorCodes.NotMaster,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({find: "c"}));
     assert.commandWorked(testDB.runCommand({find: "c"}));
@@ -53,7 +78,11 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {times: 2},
-        data: {errorCode: ErrorCodes.NotMaster, failCommands: ["find"]}
+        data: {
+            errorCode: ErrorCodes.NotMaster,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandFailedWithCode(testDB.runCommand({find: "c"}), ErrorCodes.NotMaster);
     assert.commandFailedWithCode(testDB.runCommand({find: "c"}), ErrorCodes.NotMaster);
@@ -64,7 +93,11 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {skip: 1},
-        data: {errorCode: ErrorCodes.BadValue, failCommands: ["find"]}
+        data: {
+            errorCode: ErrorCodes.BadValue,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({isMaster: 1}));
     assert.commandWorked(testDB.runCommand({buildinfo: 1}));
@@ -77,7 +110,11 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {times: 1},
-        data: {errorCode: ErrorCodes.BadValue, failCommands: ["find"]}
+        data: {
+            errorCode: ErrorCodes.BadValue,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({isMaster: 1}));
     assert.commandWorked(testDB.runCommand({buildinfo: 1}));
@@ -90,16 +127,26 @@
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: "alwaysOn",
-        data: {closeConnection: true, failCommands: ["find"]}
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.throws(() => testDB.runCommand({find: "c"}));
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
 
+    threadName = getThreadName();
+
     // Test that only commands specified in failCommands fail when closing the connection.
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: "alwaysOn",
-        data: {closeConnection: true, failCommands: ["find"]}
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({isMaster: 1}));
     assert.commandWorked(testDB.runCommand({buildinfo: 1}));
@@ -107,33 +154,34 @@
     assert.throws(() => testDB.runCommand({find: "c"}));
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
 
+    threadName = getThreadName();
+
     // Test skip when closing connection.
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {skip: 2},
-        data: {closeConnection: true, failCommands: ["find"]}
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({find: "c"}));
     assert.commandWorked(testDB.runCommand({find: "c"}));
     assert.throws(() => testDB.runCommand({find: "c"}));
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
 
-    // Test times when closing connection.
-    assert.commandWorked(adminDB.runCommand({
-        configureFailPoint: "failCommand",
-        mode: {times: 2},
-        data: {closeConnection: true, failCommands: ["find"]}
-    }));
-    assert.throws(() => testDB.runCommand({find: "c"}));
-    assert.throws(() => testDB.runCommand({find: "c"}));
-    assert.commandWorked(testDB.runCommand({find: "c"}));
-    assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
+    threadName = getThreadName();
 
     // Commands not specified in failCommands are not counted for skip.
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {skip: 1},
-        data: {closeConnection: true, failCommands: ["find"]}
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({isMaster: 1}));
     assert.commandWorked(testDB.runCommand({buildinfo: 1}));
@@ -142,11 +190,17 @@
     assert.throws(() => testDB.runCommand({find: "c"}));
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
 
+    threadName = getThreadName();
+
     // Commands not specified in failCommands are not counted for times.
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {times: 1},
-        data: {closeConnection: true, failCommands: ["find"]}
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(testDB.runCommand({isMaster: 1}));
     assert.commandWorked(testDB.runCommand({buildinfo: 1}));
@@ -155,11 +209,17 @@
     assert.commandWorked(testDB.runCommand({find: "c"}));
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
 
+    threadName = getThreadName();
+
     // Cannot fail on "configureFailPoint" command.
     assert.commandWorked(adminDB.runCommand({
         configureFailPoint: "failCommand",
         mode: {times: 1},
-        data: {errorCode: ErrorCodes.BadValue, failCommands: ["configureFailPoint"]}
+        data: {
+            errorCode: ErrorCodes.BadValue,
+            failCommands: ["configureFailPoint"],
+            threadName: threadName,
+        }
     }));
     assert.commandWorked(adminDB.runCommand({configureFailPoint: "failCommand", mode: "off"}));
 
@@ -170,6 +230,7 @@
         data: {
             writeConcernError: {code: 12345, errmsg: "hello"},
             failCommands: ['insert', 'find'],
+            threadName: threadName,
         }
     }));
     // Commands that don't support writeConcern don't tick counter.
@@ -195,6 +256,7 @@
         data: {
             writeConcernError: {code: 12345, errmsg: "hello"},
             failCommands: ['insert'],
+            threadName: threadName,
         }
     }));
     var res = testDB.runCommand({insert: "c", documents: [{_id: 'dup'}]});
diff --git a/jstests/libs/parallelTester.js b/jstests/libs/parallelTester.js
index 3bac8f0c07f..f318eaa670a 100644
--- a/jstests/libs/parallelTester.js
+++ b/jstests/libs/parallelTester.js
@@ -207,11 +207,6 @@ if (typeof _threadInject != "undefined") {
 
             // Destroys and recreates the catalog, which will interfere with other tests.
             "restart_catalog.js",
-
-            // This test makes the assumption that a command is run a certain number of times,
-            // so the test cannot be run in parallel with other tests. Also, it sets a failpoint
-            // on "find" as part of testing, causing tests run in parallel with it to possibly fail.
-            "failcommand_failpoint.js",
         ]);
 
         // The following tests cannot run when shell readMode is legacy.
diff --git a/jstests/noPassthrough/failcommand_failpoint_not_parallel.js b/jstests/noPassthrough/failcommand_failpoint_not_parallel.js
new file mode 100644
index 00000000000..2241dbe5d89
--- /dev/null
+++ b/jstests/noPassthrough/failcommand_failpoint_not_parallel.js
@@ -0,0 +1,24 @@
+(function() {
+    "use strict";
+
+    const conn = MongoRunner.runMongod();
+    assert.neq(null, conn);
+    const db = conn.getDB("test_failcommand_noparallel");
+
+    // Test times when closing connection.
+    assert.commandWorked(db.adminCommand({
+        configureFailPoint: "failCommand",
+        mode: {times: 2},
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+        }
+    }));
+    assert.throws(() => db.runCommand({find: "c"}));
+    assert.throws(() => db.runCommand({find: "c"}));
+    assert.commandWorked(db.runCommand({find: "c"}));
+    assert.commandWorked(db.adminCommand({configureFailPoint: "failCommand", mode: "off"}));
+
+    MongoRunner.stopMongod(conn);
+
+}());
diff --git a/jstests/sharding/failcommand_failpoint_not_parallel.js b/jstests/sharding/failcommand_failpoint_not_parallel.js
new file mode 100644
index 00000000000..f3e1419c770
--- /dev/null
+++ b/jstests/sharding/failcommand_failpoint_not_parallel.js
@@ -0,0 +1,22 @@
+(function() {
+    "use strict";
+
+    const st = new ShardingTest({shards: 3, mongos: 1});
+    const db = st.s.getDB("test_failcommand_noparallel");
+
+    // Test times when closing connection.
+    assert.commandWorked(st.s.adminCommand({
+        configureFailPoint: "failCommand",
+        mode: {times: 2},
+        data: {
+            closeConnection: true,
+            failCommands: ["find"],
+        }
+    }));
+    assert.throws(() => db.runCommand({find: "c"}));
+    assert.throws(() => db.runCommand({find: "c"}));
+    assert.commandWorked(db.runCommand({find: "c"}));
+    assert.commandWorked(st.s.adminCommand({configureFailPoint: "failCommand", mode: "off"}));
+
+    st.stop();
+}());