SERVER-38367 Descriptive error message for the inability to create unique indexes on arbitrary fields in sharded collections

4 files changed, 44 insertions, 21 deletions

diff --git a/jstests/sharding/index_and_collection_option_propagation.js b/jstests/sharding/index_and_collection_option_propagation.js
index ba775eb594f..7e50856014f 100644
--- a/jstests/sharding/index_and_collection_option_propagation.js
+++ b/jstests/sharding/index_and_collection_option_propagation.js
@@ -179,6 +179,20 @@ TestData.skipCheckingUUIDsConsistentAcrossCluster = true;
     assert.eq("CannotCreateIndex", res.codeName, tojson(res));
     assert.neq(null, res.errmsg, tojson(res));
 
+    // If all the non-ignorable errors reported by shards are the same, the overall command error
+    // should be set to that error.
+    res = st.s.getDB(dbName).getCollection(collName).createIndex({z: 1}, {unique: true});
+    assert.eq(res.raw[st.shard0.host].ok, 0, tojson(res));
+    assert.eq(res.raw[st.shard1.host].ok, 0, tojson(res));
+    assert.eq(null, res.raw[st.shard2.host], tojson(res));
+    assert.eq(ErrorCodes.CannotCreateIndex, res.raw[st.shard0.host].code, tojson(res));
+    assert.eq(ErrorCodes.CannotCreateIndex, res.raw[st.shard1.host].code, tojson(res));
+    assert.eq("CannotCreateIndex", res.raw[st.shard0.host].codeName, tojson(res));
+    assert.eq("CannotCreateIndex", res.raw[st.shard1.host].codeName, tojson(res));
+    assert.eq(res.code, ErrorCodes.CannotCreateIndex, tojson(res));
+    assert.eq("CannotCreateIndex", res.codeName, tojson(res));
+    assert.neq(null, res.errmsg, tojson(res));
+
     st.rs0.stopSet();
 
     // If we receive a non-ignorable error, it should be reported as the command error.
diff --git a/src/mongo/s/cluster_commands_helpers.cpp b/src/mongo/s/cluster_commands_helpers.cpp
index 981f33669ef..3d6cf6ed18b 100644
--- a/src/mongo/s/cluster_commands_helpers.cpp
+++ b/src/mongo/s/cluster_commands_helpers.cpp
@@ -187,7 +187,8 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
     StringData dbName,
     const ReadPreferenceSetting& readPref,
     Shard::RetryPolicy retryPolicy,
-    const std::vector<AsyncRequestsSender::Request>& requests) {
+    const std::vector<AsyncRequestsSender::Request>& requests,
+    const std::set<ErrorCodes::Error>& ignorableErrors) {
 
     // Send the requests.
     MultiStatementTransactionRequestsSender ars(
@@ -201,8 +202,6 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
     // Get the responses.
 
     std::vector<AsyncRequestsSender::Response> responses;  // Stores results by ShardId
-    bool atLeastOneSucceeded = false;
-    boost::optional<Status> implicitCreateErrorStatus;
 
     while (!ars.done()) {
         auto response = ars.next();
@@ -215,10 +214,6 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
             auto& responseObj = response.swResponse.getValue().data;
             status = getStatusFromCommandResult(responseObj);
 
-            if (status.isOK()) {
-                atLeastOneSucceeded = true;
-            }
-
             // Failing to establish a consistent shardVersion means no results should be examined.
             if (ErrorCodes::isStaleShardVersionError(status.code())) {
                 uassertStatusOK(status.withContext(str::stream()
@@ -244,20 +239,18 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
                 uassertStatusOK(status);
             }
 
-            if (ErrorCodes::CannotImplicitlyCreateCollection == status) {
-                implicitCreateErrorStatus = status;
+            // TODO: This should not be needed once we get better targetting with SERVER-32723.
+            // Some commands are sent with allowImplicit: false to all shards and expect only some
+            // of them to succeed.
+            if (ignorableErrors.find(ErrorCodes::CannotImplicitlyCreateCollection) ==
+                    ignorableErrors.end() &&
+                ErrorCodes::CannotImplicitlyCreateCollection == status) {
+                uassertStatusOK(status);
             }
         }
         responses.push_back(std::move(response));
     }
 
-    // TODO: This should not be needed once we get better targetting with SERVER-32723.
-    // Some commands are sent with allowImplicit: false to all shards and expect only some of
-    // them to succeed.
-    if (implicitCreateErrorStatus && !atLeastOneSucceeded) {
-        uassertStatusOK(*implicitCreateErrorStatus);
-    }
-
     return responses;
 }
 
@@ -304,7 +297,8 @@ std::vector<AsyncRequestsSender::Response> scatterGatherOnlyVersionIfUnsharded(
     const NamespaceString& nss,
     const BSONObj& cmdObj,
     const ReadPreferenceSetting& readPref,
-    Shard::RetryPolicy retryPolicy) {
+    Shard::RetryPolicy retryPolicy,
+    const std::set<ErrorCodes::Error>& ignorableErrors) {
     auto routingInfo =
         uassertStatusOK(Grid::get(opCtx)->catalogCache()->getCollectionRoutingInfo(opCtx, nss));
 
@@ -319,7 +313,7 @@ std::vector<AsyncRequestsSender::Response> scatterGatherOnlyVersionIfUnsharded(
             opCtx, nss, routingInfo, cmdObj, BSONObj(), BSONObj());
     }
 
-    return gatherResponses(opCtx, nss.db(), readPref, retryPolicy, requests);
+    return gatherResponses(opCtx, nss.db(), readPref, retryPolicy, requests, ignorableErrors);
 }
 
 AsyncRequestsSender::Response executeCommandAgainstDatabasePrimary(
diff --git a/src/mongo/s/cluster_commands_helpers.h b/src/mongo/s/cluster_commands_helpers.h
index 1e3a9d91fbb..c1dcc2f1050 100644
--- a/src/mongo/s/cluster_commands_helpers.h
+++ b/src/mongo/s/cluster_commands_helpers.h
@@ -67,7 +67,8 @@ std::vector<AsyncRequestsSender::Response> gatherResponses(
     StringData dbName,
     const ReadPreferenceSetting& readPref,
     Shard::RetryPolicy retryPolicy,
-    const std::vector<AsyncRequestsSender::Request>& requests);
+    const std::vector<AsyncRequestsSender::Request>& requests,
+    const std::set<ErrorCodes::Error>& ignorableErrors = {});
 
 /**
  * Returns a copy of 'cmdObj' with 'version' appended.
@@ -134,7 +135,8 @@ std::vector<AsyncRequestsSender::Response> scatterGatherOnlyVersionIfUnsharded(
     const NamespaceString& nss,
     const BSONObj& cmdObj,
     const ReadPreferenceSetting& readPref,
-    Shard::RetryPolicy retryPolicy);
+    Shard::RetryPolicy retryPolicy,
+    const std::set<ErrorCodes::Error>& ignorableErrors = {});
 
 /**
  * Utility for dispatching commands against the primary of a database and attach the appropriate
diff --git a/src/mongo/s/commands/cluster_create_indexes_cmd.cpp b/src/mongo/s/commands/cluster_create_indexes_cmd.cpp
index c4a4f8dc9c4..65ac1d1abcc 100644
--- a/src/mongo/s/commands/cluster_create_indexes_cmd.cpp
+++ b/src/mongo/s/commands/cluster_create_indexes_cmd.cpp
@@ -32,6 +32,7 @@
 #include "mongo/platform/basic.h"
 
 #include "mongo/db/commands.h"
+#include "mongo/rpc/get_status_from_command_result.h"
 #include "mongo/s/cluster_commands_helpers.h"
 #include "mongo/util/log.h"
 
@@ -77,7 +78,19 @@ public:
             nss,
             CommandHelpers::filterCommandRequestForPassthrough(cmdObj),
             ReadPreferenceSetting::get(opCtx),
-            Shard::RetryPolicy::kNoRetry);
+            Shard::RetryPolicy::kNoRetry,
+            {ErrorCodes::CannotImplicitlyCreateCollection});
+
+        if (std::all_of(shardResponses.begin(), shardResponses.end(), [](const auto& response) {
+                return response.swResponse.getStatus().isOK() &&
+                    getStatusFromCommandResult(response.swResponse.getValue().data) ==
+                    ErrorCodes::CannotImplicitlyCreateCollection;
+            })) {
+            // Propagate the ExtraErrorInfo from the first response.
+            uassertStatusOK(
+                getStatusFromCommandResult(shardResponses.front().swResponse.getValue().data));
+        }
+
         return appendRawResponses(opCtx,
                                   &errmsg,
                                   &output,