From 242deee7c584613d0cd8f5bc9e10eea73bd18aad Mon Sep 17 00:00:00 2001 From: Esha Maharishi Date: Fri, 10 May 2019 14:02:09 -0400 Subject: SERVER-38367 Descriptive error message for the inability to create unique indexes on arbitrary fields in sharded collections --- .../index_and_collection_option_propagation.js | 14 ++++++++++ src/mongo/s/cluster_commands_helpers.cpp | 30 +++++++++------------- src/mongo/s/cluster_commands_helpers.h | 6 +++-- .../s/commands/cluster_create_indexes_cmd.cpp | 15 ++++++++++- 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 gatherResponses( StringData dbName, const ReadPreferenceSetting& readPref, Shard::RetryPolicy retryPolicy, - const std::vector& requests) { + const std::vector& requests, + const std::set& ignorableErrors) { // Send the requests. MultiStatementTransactionRequestsSender ars( @@ -201,8 +202,6 @@ std::vector gatherResponses( // Get the responses. std::vector responses; // Stores results by ShardId - bool atLeastOneSucceeded = false; - boost::optional implicitCreateErrorStatus; while (!ars.done()) { auto response = ars.next(); @@ -215,10 +214,6 @@ std::vector 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 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 scatterGatherOnlyVersionIfUnsharded( const NamespaceString& nss, const BSONObj& cmdObj, const ReadPreferenceSetting& readPref, - Shard::RetryPolicy retryPolicy) { + Shard::RetryPolicy retryPolicy, + const std::set& ignorableErrors) { auto routingInfo = uassertStatusOK(Grid::get(opCtx)->catalogCache()->getCollectionRoutingInfo(opCtx, nss)); @@ -319,7 +313,7 @@ std::vector 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 gatherResponses( StringData dbName, const ReadPreferenceSetting& readPref, Shard::RetryPolicy retryPolicy, - const std::vector& requests); + const std::vector& requests, + const std::set& ignorableErrors = {}); /** * Returns a copy of 'cmdObj' with 'version' appended. @@ -134,7 +135,8 @@ std::vector scatterGatherOnlyVersionIfUnsharded( const NamespaceString& nss, const BSONObj& cmdObj, const ReadPreferenceSetting& readPref, - Shard::RetryPolicy retryPolicy); + Shard::RetryPolicy retryPolicy, + const std::set& 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, -- cgit v1.2.1