SERVER-63531 Correct error to indicate that non-voting members can be included in commitQuorum

(cherry picked from commit 303a724f519e8d45d3b5d1d31c1ac509cfabf08c)

SERVER-63531 Prevent all buildIndexes:false nodes from being included in commitQuorum

Previously only voting buildIndexes:false nodes were excluded from commitQuorum. But all buildIndexes:false nodes,
including non-voting ones, should be excluded.

(cherry picked from commit 5f6cd9a3c69faef393b79e18b118b29f2cce0123)

3 files changed, 100 insertions, 14 deletions

diff --git a/jstests/noPassthrough/commit_quorum_voting_nodes.js b/jstests/noPassthrough/commit_quorum_voting_nodes.js
new file mode 100644
index 00000000000..87e7b1ce27a
--- /dev/null
+++ b/jstests/noPassthrough/commit_quorum_voting_nodes.js
@@ -0,0 +1,83 @@
+/**
+ * Tests that index build commitQuorum can include non-voting data-bearing nodes.
+ *
+ * @tags: [
+ *   requires_journaling,
+ *   requires_persistence,
+ *   requires_replication,
+ * ]
+ */
+
+(function() {
+const replSet = new ReplSetTest({
+    nodes: [
+        {
+            rsConfig: {
+                tags: {es: 'dc1'},
+            }
+        },
+        {
+            rsConfig: {
+                tags: {es: 'dc2'},
+            }
+        },
+        {
+            // Analytics node with zero votes should be included in a commitQuorum.
+            rsConfig: {
+                priority: 0,
+                votes: 0,
+                tags: {es: 'analytics'},
+            },
+        },
+    ],
+});
+
+replSet.startSet();
+replSet.initiate();
+
+const primary = replSet.getPrimary();
+const config = primary.getDB('local').system.replset.findOne();
+
+// Create a custom write concern for both nodes with the 'es' tag. We will expect this to be usable
+// as an option to commitQuorum.
+config.settings = {
+    getLastErrorModes: {ESP: {"es": 3}}
+};
+config.version++;
+assert.commandWorked(primary.getDB("admin").runCommand({replSetReconfig: config}));
+
+const db = replSet.getPrimary().getDB('test');
+const coll = db['coll'];
+assert.commandWorked(coll.insert({a: 1}));
+
+// Shut the analytics node down so that it cannot contribute to the commitQuorum.
+const analyticsNodeId = 2;
+replSet.stop(analyticsNodeId, {forRestart: true});
+
+// The default commitQuorum should not include the non-voting analytics node.
+assert.commandWorked(coll.createIndex({a: 1}));
+coll.dropIndexes();
+
+// The explicit "votingMembers" should not include the non-voting analytics node.
+assert.commandWorked(coll.createIndex({a: 1}, {}, "votingMembers"));
+coll.dropIndexes();
+
+// Restart the analytics node down so that it can contribute to the commitQuorum.
+replSet.start(analyticsNodeId, {}, true /* restart */);
+
+// This should include the non-voting analytics node.
+const nNodes = replSet.nodeList().length;
+assert.commandWorked(coll.createIndex({a: 1}, {}, nNodes));
+coll.dropIndexes();
+
+// This custom tag should include the analytics node.
+assert.commandWorked(coll.createIndex({a: 1}, {}, "ESP"));
+coll.dropIndexes();
+
+// Not enough data-bearing nodes to satisfy commit quorum.
+assert.commandFailedWithCode(coll.createIndex({a: 1}, {}, nNodes + 1),
+                             ErrorCodes.UnsatisfiableCommitQuorum);
+coll.dropIndexes();
+
+replSet.stopSet();
+})();
diff --git a/jstests/replsets/buildindexes_false_commit_quorum.js b/jstests/replsets/buildindexes_false_commit_quorum.js
index 2caa757d4d3..3811b477778 100644
--- a/jstests/replsets/buildindexes_false_commit_quorum.js
+++ b/jstests/replsets/buildindexes_false_commit_quorum.js
@@ -21,7 +21,9 @@ const replTest = new ReplSetTest({
     nodes: [
         {},
         {},
+        {},
         {rsConfig: {priority: 0, buildIndexes: false}},
+        {rsConfig: {priority: 0, votes: 0, buildIndexes: false}},
     ]
 });
 replTest.startSet();
@@ -45,12 +47,12 @@ assert.commandFailedWithCode(primaryDb.runCommand({
 }),
                              ErrorCodes.UnsatisfiableCommitQuorum);
 
-// With a commit quorum that includes all nodes, the quorum is unsatisfiable for the same reason as
-// 'votingMembers'.
+// With a commit quorum that includes 4 nodes, the quorum is unsatisfiable because it includes a
+// buildIndexes: false node.
 assert.commandFailedWithCode(primaryDb.runCommand({
     createIndexes: collName,
     indexes: [{key: {y: 1}, name: 'y_1_commitQuorum_3'}],
-    commitQuorum: 3,
+    commitQuorum: 4,
 }),
                              ErrorCodes.UnsatisfiableCommitQuorum);
 
@@ -72,7 +74,9 @@ replTest.getSecondaries().forEach((conn) => {
 });
 
 IndexBuildTest.assertIndexes(secondaryDbs[0][collName], 2, ['_id_', indexName]);
-IndexBuildTest.assertIndexes(secondaryDbs[1][collName], 1, ['_id_']);
+IndexBuildTest.assertIndexes(secondaryDbs[1][collName], 2, ['_id_', indexName]);
+IndexBuildTest.assertIndexes(secondaryDbs[2][collName], 1, ['_id_']);
+IndexBuildTest.assertIndexes(secondaryDbs[3][collName], 1, ['_id_']);
 
 replTest.stopSet();
 }());
diff --git a/src/mongo/db/repl/topology_coordinator.cpp b/src/mongo/db/repl/topology_coordinator.cpp
index 26bbc847a93..86f8b335bdb 100644
--- a/src/mongo/db/repl/topology_coordinator.cpp
+++ b/src/mongo/db/repl/topology_coordinator.cpp
@@ -3516,16 +3516,16 @@ Status TopologyCoordinator::checkIfCommitQuorumCanBeSatisfied(
         }
     }
 
-    bool votingBuildIndexesFalseNodes = false;
+    bool buildIndexesFalseNodes = false;
     for (auto&& member : _rsConfig.members()) {
         // Only count data-bearing nodes.
         if (member.isArbiter()) {
             continue;
         }
 
-        // Only count voting nodes that build indexes.
-        if (member.isVoter() && !member.shouldBuildIndexes()) {
-            votingBuildIndexesFalseNodes = true;
+        // Only count nodes that build indexes.
+        if (!member.shouldBuildIndexes()) {
+            buildIndexesFalseNodes = true;
             continue;
         }
 
@@ -3535,18 +3535,17 @@ Status TopologyCoordinator::checkIfCommitQuorumCanBeSatisfied(
         }
     }
 
-    // Voting, buildIndexes:false nodes can be included in a commitQuorum but never actually build
+    // buildIndexes:false should not be included in a commitQuorum because they never actually build
     // indexes and vote to commit. Provide a helpful error message to prevent users from starting
     // index builds that will never commit.
-    if (votingBuildIndexesFalseNodes) {
+    if (buildIndexesFalseNodes) {
         return {ErrorCodes::UnsatisfiableCommitQuorum,
-                str::stream()
-                    << "Commit quorum cannot depend on voting buildIndexes:false nodes; "
-                    << "use a commit quorum that excludes these nodes or do not give them votes"};
+                str::stream() << "Commit quorum cannot depend on buildIndexes:false nodes; "
+                              << "use a commit quorum that excludes these nodes"};
     }
 
     return {ErrorCodes::UnsatisfiableCommitQuorum,
-            "Not enough data-bearing voting nodes to satisfy commit quorum"};
+            "Not enough data-bearing nodes to satisfy commit quorum"};
 }
 
 }  // namespace repl