summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorEsha Maharishi <esha.maharishi@mongodb.com>2019-05-06 13:08:55 -0400
committerEsha Maharishi <esha.maharishi@mongodb.com>2019-05-10 14:09:08 -0400
commit87c32b0ee8f862b330908fb1653f851aee2a6fa4 (patch)
treed69b3a78f74e4bcb93e413adf1e5d2ece2629e32 /src
parentc34bc93a15cbb6fe8b222f12afac5adbab6f0737 (diff)
downloadmongo-87c32b0ee8f862b330908fb1653f851aee2a6fa4.tar.gz
SERVER-38369 Only surface a 'request doesn't allow collection to be created implicitly' error if no shard has the collection
Diffstat (limited to 'src')
-rw-r--r--src/mongo/s/SConscript13
-rw-r--r--src/mongo/s/append_raw_responses_test.cpp541
-rw-r--r--src/mongo/s/cluster_commands_helpers.cpp153
-rw-r--r--src/mongo/s/cluster_commands_helpers.h2
4 files changed, 626 insertions, 83 deletions
diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript
index 88db8654426..3566cbbf4b3 100644
--- a/src/mongo/s/SConscript
+++ b/src/mongo/s/SConscript
@@ -484,6 +484,19 @@ env.Library(
)
env.CppUnitTest(
+ target='cluster_commands_helpers_test',
+ source=[
+ 'append_raw_responses_test.cpp',
+ ],
+ LIBDEPS=[
+ '$BUILD_DIR/mongo/db/auth/authmocks',
+ '$BUILD_DIR/mongo/s/catalog/sharding_catalog_client_mock',
+ 'shard_server_test_fixture',
+ 'sharding_router_api',
+ ]
+)
+
+env.CppUnitTest(
target='balancer_configuration_test',
source=[
'balancer_configuration_test.cpp',
diff --git a/src/mongo/s/append_raw_responses_test.cpp b/src/mongo/s/append_raw_responses_test.cpp
new file mode 100644
index 00000000000..791949a8672
--- /dev/null
+++ b/src/mongo/s/append_raw_responses_test.cpp
@@ -0,0 +1,541 @@
+/**
+ * Copyright (C) 2018-present MongoDB, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the Server Side Public License, version 1,
+ * as published by MongoDB, Inc.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * Server Side Public License for more details.
+ *
+ * You should have received a copy of the Server Side Public License
+ * along with this program. If not, see
+ * <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ * As a special exception, the copyright holders give permission to link the
+ * code of portions of this program with the OpenSSL library under certain
+ * conditions as described in each individual source file and distribute
+ * linked combinations including the program with the OpenSSL library. You
+ * must comply with the Server Side Public License in all respects for
+ * all of the code used other than as permitted herein. If you modify file(s)
+ * with this exception, you may extend this exception to your version of the
+ * file(s), but you are not obligated to do so. If you do not wish to do so,
+ * delete this exception statement from your version. If you delete this
+ * exception statement from all source files in the program, then also delete
+ * it in the license file.
+ */
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kSharding
+
+#include "mongo/unittest/unittest.h"
+
+#include "mongo/client/remote_command_targeter_mock.h"
+#include "mongo/db/commands.h"
+#include "mongo/rpc/get_status_from_command_result.h"
+#include "mongo/s/async_requests_sender.h"
+#include "mongo/s/catalog/sharding_catalog_client_mock.h"
+#include "mongo/s/catalog/type_shard.h"
+#include "mongo/s/cluster_commands_helpers.h"
+#include "mongo/s/shard_server_test_fixture.h"
+
+namespace mongo {
+namespace {
+
+// Have two each of ignorable and non-ignorable errors, so we can test shards returning different
+// ignorable and different non-ignorable errors.
+const ErrorCodes::Error kIgnorableError1{ErrorCodes::NamespaceNotFound};
+const ErrorCodes::Error kIgnorableError2{ErrorCodes::NamespaceNotFound};
+const ErrorCodes::Error kNonIgnorableError1{ErrorCodes::HostUnreachable};
+const ErrorCodes::Error kNonIgnorableError2{ErrorCodes::HostUnreachable};
+
+const Status kIgnorableError1Status{kIgnorableError1, "dummy"};
+const Status kIgnorableError2Status{kIgnorableError2, "dummy"};
+const Status kNonIgnorableError1Status{kNonIgnorableError1, "dummy"};
+const Status kNonIgnorableError2Status{kNonIgnorableError2, "dummy"};
+
+const Status kWriteConcernError1Status{ErrorCodes::WriteConcernFailed, "dummy"};
+const Status kWriteConcernError2Status{ErrorCodes::UnsatisfiableWriteConcern, "dummy"};
+
+executor::RemoteCommandResponse kOkResponse{BSON("ok" << 1), Milliseconds(0)};
+
+executor::RemoteCommandResponse makeErrorResponse(const Status& errorStatus) {
+ invariant(!errorStatus.isOK());
+ BSONObjBuilder res;
+ CommandHelpers::appendCommandStatusNoThrow(res, errorStatus);
+ return {res.obj(), Milliseconds(0)};
+}
+
+executor::RemoteCommandResponse makeWriteConcernErrorResponse(
+ const Status& writeConcernErrorStatus) {
+ invariant(!writeConcernErrorStatus.isOK());
+ BSONObjBuilder res;
+ WriteConcernErrorDetail wcError;
+ wcError.setStatus(writeConcernErrorStatus);
+ res.append("ok", 1);
+ res.append("writeConcernError", wcError.toBSON());
+ return {res.obj(), Milliseconds(0)};
+}
+
+HostAndPort makeHostAndPort(const ShardId& shardId) {
+ return HostAndPort(str::stream() << shardId << ":123");
+}
+
+} // namespace
+
+class AppendRawResponsesTest : public ShardServerTestFixture {
+protected:
+ /**
+ * Runs 'appendRawResponses' and asserts that the top-level status matches 'expectedStatus', the
+ * 'raw' sub-object contains the shards in 'expectedShardsInRawSubObj', and the writeConcern
+ * status matches 'expectedWriteConcernStatus'.
+ */
+ void runAppendRawResponsesExpect(
+ const std::vector<AsyncRequestsSender::Response>& shardResponses,
+ const Status& expectedStatus,
+ const std::vector<ShardId>& expectedShardsInRawSubObj,
+ const Status& expectedWriteConcernStatus = Status::OK()) {
+ BSONObjBuilder result;
+ std::string errmsg;
+
+ const auto ok = appendRawResponses(operationContext(),
+ &errmsg,
+ &result,
+ shardResponses,
+ {kIgnorableError1, kIgnorableError2});
+
+ if (!errmsg.empty()) {
+ CommandHelpers::appendSimpleCommandStatus(result, ok, errmsg);
+ }
+ const auto resultObj = result.obj();
+
+ // Check the top-level status.
+ if (expectedStatus.isOK()) {
+ ASSERT_FALSE(resultObj.hasField("code"));
+ ASSERT_FALSE(resultObj.hasField("codeName"));
+ ASSERT(errmsg.empty());
+ } else {
+ ASSERT_EQ(expectedStatus.code(), resultObj.getField("code").Int());
+ ASSERT_EQ(ErrorCodes::errorString(expectedStatus.code()),
+ resultObj.getField("codeName").String());
+ ASSERT_EQ(expectedStatus.reason(), errmsg);
+ }
+
+ // Check the 'raw' sub-object.
+ const auto rawSubObj = resultObj.getField("raw").Obj();
+ ASSERT_EQ(rawSubObj.nFields(), int(expectedShardsInRawSubObj.size()));
+ for (const auto& shardId : expectedShardsInRawSubObj) {
+ const auto shardConnStr =
+ ConnectionString::forReplicaSet(shardId.toString(), {makeHostAndPort(shardId)})
+ .toString();
+ ASSERT(rawSubObj.hasField(shardConnStr));
+ }
+
+ // Check for a writeConcern error.
+ if (expectedWriteConcernStatus.isOK()) {
+ ASSERT(resultObj.getField("writeConcernError").eoo());
+ } else {
+ ASSERT_EQ(expectedWriteConcernStatus,
+ getWriteConcernStatusFromCommandResult(resultObj));
+ }
+ }
+
+ void setUp() override {
+ ShardServerTestFixture::setUp();
+
+ for (const auto& shardId : kShardIdList) {
+ auto shardTargeter = RemoteCommandTargeterMock::get(
+ uassertStatusOK(shardRegistry()->getShard(operationContext(), shardId))
+ ->getTargeter());
+ shardTargeter->setConnectionStringReturnValue(
+ ConnectionString::forReplicaSet(shardId.toString(), {makeHostAndPort(shardId)}));
+ }
+ }
+
+ std::unique_ptr<ShardingCatalogClient> makeShardingCatalogClient(
+ std::unique_ptr<DistLockManager> distLockManager) override {
+
+ class StaticCatalogClient final : public ShardingCatalogClientMock {
+ public:
+ StaticCatalogClient(std::vector<ShardId> shardIds)
+ : ShardingCatalogClientMock(nullptr), _shardIds(std::move(shardIds)) {}
+
+ StatusWith<repl::OpTimeWith<std::vector<ShardType>>> getAllShards(
+ OperationContext* opCtx, repl::ReadConcernLevel readConcern) override {
+ std::vector<ShardType> shardTypes;
+ for (const auto& shardId : _shardIds) {
+ const ConnectionString cs = ConnectionString::forReplicaSet(
+ shardId.toString(), {makeHostAndPort(shardId)});
+ ShardType sType;
+ sType.setName(cs.getSetName());
+ sType.setHost(cs.toString());
+ shardTypes.push_back(std::move(sType));
+ };
+ return repl::OpTimeWith<std::vector<ShardType>>(shardTypes);
+ }
+
+ private:
+ const std::vector<ShardId> _shardIds;
+ };
+
+ return stdx::make_unique<StaticCatalogClient>(kShardIdList);
+ }
+
+ const ShardId kShard1{"s1"};
+ const ShardId kShard2{"s2"};
+ const ShardId kShard3{"s3"};
+ const ShardId kShard4{"s4"};
+ const ShardId kShard5{"s5"};
+ const std::vector<ShardId> kShardIdList{kShard1, kShard2, kShard3, kShard4, kShard5};
+};
+
+//
+// No errors
+//
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnSuccess) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kOkResponse, makeHostAndPort(kShard3)}};
+
+ runAppendRawResponsesExpect(shardResponses, Status::OK(), {kShard1, kShard2, kShard3});
+}
+
+//
+// Only write concern errors
+//
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnSuccessOneWithWriteConcernError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1,
+ makeWriteConcernErrorResponse(kWriteConcernError1Status),
+ makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kOkResponse, makeHostAndPort(kShard3)}};
+
+ runAppendRawResponsesExpect(
+ shardResponses, Status::OK(), {kShard1, kShard2, kShard3}, kWriteConcernError1Status);
+}
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnSuccessMoreThanOneWithWriteConcernError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1,
+ makeWriteConcernErrorResponse(kWriteConcernError1Status),
+ makeHostAndPort(kShard1)},
+ {kShard2,
+ makeWriteConcernErrorResponse(kWriteConcernError2Status),
+ makeHostAndPort(kShard2)},
+ {kShard3, kOkResponse, makeHostAndPort(kShard3)}};
+
+ // The first writeConcern error is reported.
+ runAppendRawResponsesExpect(
+ shardResponses, Status::OK(), {kShard1, kShard2, kShard3}, kWriteConcernError1Status);
+}
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnSuccessAllWithWriteConcernError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1,
+ makeWriteConcernErrorResponse(kWriteConcernError1Status),
+ makeHostAndPort(kShard1)},
+ {kShard2,
+ makeWriteConcernErrorResponse(kWriteConcernError2Status),
+ makeHostAndPort(kShard2)},
+ {kShard3,
+ makeWriteConcernErrorResponse(kWriteConcernError1Status),
+ makeHostAndPort(kShard3)}};
+
+ // The first writeConcern error is reported.
+ runAppendRawResponsesExpect(
+ shardResponses, Status::OK(), {kShard1, kShard2, kShard3}, kWriteConcernError1Status);
+}
+
+//
+// Errors *sending* the requests.
+//
+
+TEST_F(AppendRawResponsesTest, AllAttemptsToSendRequestsReturnSameIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, kIgnorableError1Status, boost::none},
+ {kShard3, kIgnorableError1Status, boost::none}};
+
+ // The ignorable error gets promoted to a non-ignorable error.
+ runAppendRawResponsesExpect(
+ shardResponses, kIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestOfAttemptsToSendRequestsReturnSameIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kIgnorableError1Status, boost::none},
+ {kShard4, kIgnorableError1Status, boost::none}};
+
+ // The ignorable errors are ignored.
+ runAppendRawResponsesExpect(shardResponses, Status::OK(), {kShard1, kShard2});
+}
+
+TEST_F(AppendRawResponsesTest, AttemptsToSendRequestsReturnDifferentIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, kIgnorableError2Status, boost::none},
+ {kShard3, kIgnorableError1Status, boost::none}};
+
+ // The *first* ignorable error gets promoted to a non-ignorable error.
+ runAppendRawResponsesExpect(
+ shardResponses, kIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestOfAttemptsToSendRequestsReturnDifferentIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kIgnorableError1Status, boost::none},
+ {kShard4, kIgnorableError2Status, boost::none}};
+
+ // The ignorable errors are ignored.
+ runAppendRawResponsesExpect(shardResponses, Status::OK(), {kShard1, kShard2});
+}
+
+TEST_F(AppendRawResponsesTest, AllAttemptsToSendRequestsReturnSameNonIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kNonIgnorableError1Status, boost::none},
+ {kShard2, kNonIgnorableError1Status, boost::none},
+ {kShard3, kNonIgnorableError1Status, boost::none}};
+
+ // The non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestOfAttemptsToSendRequestsReturnSameNonIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, kNonIgnorableError1Status, boost::none}};
+
+ // The non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3, kShard4});
+}
+
+TEST_F(AppendRawResponsesTest, AttemptsToSendRequestsReturnDifferentNonIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kNonIgnorableError1Status, boost::none},
+ {kShard2, kNonIgnorableError2Status, boost::none},
+ {kShard3, kNonIgnorableError1Status, boost::none}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestOfAttemptsToSendRequestsReturnDifferentNonIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, kNonIgnorableError2Status, boost::none}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3, kShard4});
+}
+
+TEST_F(AppendRawResponsesTest, AllAttemptsToSendRequestsReturnErrorsSomeIgnorableSomeNonIgnorable) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, kIgnorableError2Status, boost::none},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, kNonIgnorableError2Status, boost::none}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(shardResponses, kNonIgnorableError1Status, {kShard3, kShard4});
+}
+
+TEST_F(
+ AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestOfAttemptsToSendRequestsReturnErrorsSomeIgnorableSomeNonIgnorable) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, kNonIgnorableError2Status, boost::none}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard2, kShard3, kShard4});
+}
+
+//
+// Errors *processing* the requests.
+//
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnSameIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard1)},
+ {kShard2, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard3)}};
+
+ // The ignorable error gets promoted to a non-ignorable error.
+ runAppendRawResponsesExpect(
+ shardResponses, kIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest, SomeShardsReturnSuccessRestReturnSameIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard3)},
+ {kShard4, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard4)}};
+
+ // The ignorable errors are ignored.
+ runAppendRawResponsesExpect(shardResponses, Status::OK(), {kShard1, kShard2});
+}
+
+TEST_F(AppendRawResponsesTest, ShardsReturnDifferentIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard1)},
+ {kShard2, makeErrorResponse(kIgnorableError2Status), makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard3)}};
+
+ // The *first* ignorable error gets promoted to a non-ignorable error.
+ runAppendRawResponsesExpect(
+ shardResponses, kIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest, SomeShardsReturnSuccessRestReturnDifferentIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard3)},
+ {kShard4, makeErrorResponse(kIgnorableError2Status), makeHostAndPort(kShard4)}};
+
+ // The ignorable errors are ignored.
+ runAppendRawResponsesExpect(shardResponses, Status::OK(), {kShard1, kShard2});
+}
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnSameNonIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard1)},
+ {kShard2, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard3)}};
+
+ // The non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest, SomeShardsReturnSuccessRestReturnSameNonIgnorableError) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard3)},
+ {kShard4, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard4)}};
+
+ // The non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3, kShard4});
+}
+
+TEST_F(AppendRawResponsesTest, ShardsReturnDifferentNonIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard1)},
+ {kShard2, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard3)}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3});
+}
+
+TEST_F(AppendRawResponsesTest, SomeShardsReturnSuccessRestReturnDifferentNonIgnorableErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kOkResponse, makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard3)},
+ {kShard4, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard4)}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard1, kShard2, kShard3, kShard4});
+}
+
+TEST_F(AppendRawResponsesTest, AllShardsReturnErrorsSomeIgnorableSomeNonIgnorable) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard1)},
+ {kShard2, makeErrorResponse(kIgnorableError2Status), makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard3)},
+ {kShard4, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard4)}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(shardResponses, kNonIgnorableError1Status, {kShard3, kShard4});
+}
+
+TEST_F(AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestReturnErrorsSomeIgnorableSomeNonIgnorable) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, makeErrorResponse(kIgnorableError1Status), makeHostAndPort(kShard1)},
+ {kShard2, kOkResponse, makeHostAndPort(kShard2)},
+ {kShard3, makeErrorResponse(kNonIgnorableError1Status), makeHostAndPort(kShard3)},
+ {kShard4, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard4)}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard2, kShard3, kShard4});
+}
+
+// Mix of errors *sending* and *processing* the requests.
+
+TEST_F(AppendRawResponsesTest,
+ AllShardsReturnErrorsMixOfErrorsSendingRequestsAndErrorsProcessingRequests) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, makeErrorResponse(kIgnorableError2Status), makeHostAndPort(kShard2)},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard4)}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(shardResponses, kNonIgnorableError1Status, {kShard3, kShard4});
+}
+
+TEST_F(
+ AppendRawResponsesTest,
+ SomeShardsReturnSuccessRestReturnErrorsMixOfErrorsSendingRequestsAndErrorsProcessingRequests) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, makeErrorResponse(kIgnorableError2Status), makeHostAndPort(kShard2)},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard4)},
+ {kShard5, kOkResponse, HostAndPort("e:1")}};
+
+ // The first non-ignorable error is returned.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard3, kShard4, kShard5});
+}
+
+// Mix of errors sending or processing the requests *and* writeConcern error.
+
+TEST_F(AppendRawResponsesTest, SomeShardsReturnSuccessWithWriteConcernErrorRestReturnMixOfErrors) {
+ std::vector<AsyncRequestsSender::Response> shardResponses{
+ {kShard1, kIgnorableError1Status, boost::none},
+ {kShard2, makeErrorResponse(kIgnorableError2Status), makeHostAndPort(kShard2)},
+ {kShard3, kNonIgnorableError1Status, boost::none},
+ {kShard4, makeErrorResponse(kNonIgnorableError2Status), makeHostAndPort(kShard4)},
+ {kShard5,
+ makeWriteConcernErrorResponse(kWriteConcernError1Status),
+ makeHostAndPort(kShard5)}};
+
+ // The first non-ignorable error is returned, and no writeConcern error is reported at the top
+ // level.
+ runAppendRawResponsesExpect(
+ shardResponses, kNonIgnorableError1Status, {kShard3, kShard4, kShard5});
+}
+
+} // namespace mongo
diff --git a/src/mongo/s/cluster_commands_helpers.cpp b/src/mongo/s/cluster_commands_helpers.cpp
index 5c498273192..981f33669ef 100644
--- a/src/mongo/s/cluster_commands_helpers.cpp
+++ b/src/mongo/s/cluster_commands_helpers.cpp
@@ -342,111 +342,100 @@ AsyncRequestsSender::Response executeCommandAgainstDatabasePrimary(
bool appendRawResponses(OperationContext* opCtx,
std::string* errmsg,
BSONObjBuilder* output,
- std::vector<AsyncRequestsSender::Response> shardResponses,
- std::set<ErrorCodes::Error> ignoredErrors) {
- // Always include ShardNotFound as an ignored error, since this node may not have realized a
+ const std::vector<AsyncRequestsSender::Response>& shardResponses,
+ std::set<ErrorCodes::Error> ignorableErrors) {
+ // Always include ShardNotFound as an ignorable error, since this node may not have realized a
// shard has been removed.
- ignoredErrors.insert(ErrorCodes::ShardNotFound);
+ ignorableErrors.insert(ErrorCodes::ShardNotFound);
- BSONObjBuilder subobj; // Stores raw responses by ConnectionString
+ std::vector<std::pair<ShardId, BSONObj>> successResponsesReceived;
+ std::vector<std::pair<ShardId, Status>> ignorableErrorsReceived;
+ std::vector<std::pair<ShardId, Status>> nonIgnorableErrorsReceived;
- // Stores all errors; we will remove ignoredErrors later if some shard returned success.
- std::vector<std::pair<std::string, Status>> errors; // Stores errors by ConnectionString
+ boost::optional<std::pair<ShardId, BSONElement>> firstWriteConcernErrorReceived;
- BSONElement wcErrorElem; // Stores the first writeConcern error we encounter
- ShardId wcErrorShardId; // Stores the shardId for the first writeConcern error we encounter
- bool hasWCError = false; // Whether we have encountered a writeConcern error yet
+ const auto processError = [&](const ShardId& shardId, const Status& status) {
+ invariant(!status.isOK());
+ if (ignorableErrors.find(status.code()) != ignorableErrors.end()) {
+ ignorableErrorsReceived.emplace_back(std::move(shardId), std::move(status));
+ return;
+ }
+ nonIgnorableErrorsReceived.emplace_back(shardId, status);
+ };
+ // Do a pass through all the received responses and group them into success, ignorable, and
+ // non-ignorable.
for (const auto& shardResponse : shardResponses) {
- // Get the Shard object in order to get the shard's ConnectionString.
- const auto swShard =
- Grid::get(opCtx)->shardRegistry()->getShard(opCtx, shardResponse.shardId);
- if (ErrorCodes::ShardNotFound == swShard.getStatus().code()) {
- // Store the error by ShardId, since we cannot know the shard connection string, and it
- // is only used for reporting.
- errors.push_back(std::make_pair(shardResponse.shardId.toString(), swShard.getStatus()));
- continue;
- }
- const auto shard = uassertStatusOK(swShard);
- const auto shardConnStr = shard->getConnString().toString();
+ const auto& shardId = shardResponse.shardId;
- Status sendStatus = shardResponse.swResponse.getStatus();
+ const auto sendStatus = shardResponse.swResponse.getStatus();
if (!sendStatus.isOK()) {
- // Convert the error status back into the form of a command result and append it as the
- // raw response.
- BSONObjBuilder statusObjBob;
- CommandHelpers::appendCommandStatusNoThrow(statusObjBob, sendStatus);
- subobj.append(shardConnStr, statusObjBob.obj());
-
- errors.push_back(std::make_pair(shardConnStr, sendStatus));
+ processError(shardId, sendStatus);
continue;
}
- // Got a response from the shard.
-
- auto& resObj = shardResponse.swResponse.getValue().data;
-
- // Append the shard's raw response.
- subobj.append(shardConnStr, CommandHelpers::filterCommandReplyForPassthrough(resObj));
-
- auto commandStatus = getStatusFromCommandResult(resObj);
+ const auto& resObj = shardResponse.swResponse.getValue().data;
+ const auto commandStatus = getStatusFromCommandResult(resObj);
if (!commandStatus.isOK()) {
- errors.push_back(std::make_pair(shardConnStr, std::move(commandStatus)));
+ processError(shardId, commandStatus);
+ continue;
}
- // Report the first writeConcern error we see.
- if (!hasWCError && (wcErrorElem = resObj["writeConcernError"])) {
- wcErrorShardId = shardResponse.shardId;
- hasWCError = true;
+ if (!firstWriteConcernErrorReceived && resObj["writeConcernError"]) {
+ firstWriteConcernErrorReceived.emplace(shardId, resObj["writeConcernError"]);
}
- }
- output->append("raw", subobj.done());
-
- if (hasWCError) {
- appendWriteConcernErrorToCmdResponse(wcErrorShardId, wcErrorElem, *output);
+ successResponsesReceived.emplace_back(shardId, resObj);
}
- // If any shard returned success, filter out ignored errors
- bool someShardReturnedOK = (errors.size() != shardResponses.size());
+ // If all shards reported ignorable errors, promote them all to non-ignorable errors.
+ if (ignorableErrorsReceived.size() == shardResponses.size()) {
+ invariant(nonIgnorableErrorsReceived.empty());
+ nonIgnorableErrorsReceived = std::move(ignorableErrorsReceived);
+ }
- BSONObjBuilder errorBob;
- int commonErrCode = -1;
- auto it = errors.begin();
- while (it != errors.end()) {
- if (someShardReturnedOK && ignoredErrors.find(it->second.code()) != ignoredErrors.end()) {
- // Ignore the error.
- it = errors.erase(it);
- } else {
- errorBob.append(it->first, it->second.reason());
- if (commonErrCode == -1) {
- commonErrCode = it->second.code();
- } else if (commonErrCode != it->second.code()) {
- commonErrCode = 0;
- }
- ++it;
+ // Append a 'raw' field containing the success responses and non-ignorable error responses.
+ BSONObjBuilder rawShardResponses;
+ const auto appendRawResponse = [&](const ShardId& shardId, const BSONObj& response) {
+ // Try to report the response by the shard's full connection string.
+ try {
+ const auto shardConnString =
+ uassertStatusOK(Grid::get(opCtx)->shardRegistry()->getShard(opCtx, shardId))
+ ->getConnString();
+ rawShardResponses.append(shardConnString.toString(),
+ CommandHelpers::filterCommandReplyForPassthrough(response));
+ } catch (const ExceptionFor<ErrorCodes::ShardNotFound>&) {
+ // If we could not get the shard's connection string, fall back to reporting the
+ // response by shard id.
+ rawShardResponses.append(shardId, response);
}
+ };
+ for (const auto& success : successResponsesReceived) {
+ appendRawResponse(success.first, success.second);
}
- BSONObj errobj = errorBob.obj();
-
- if (!errobj.isEmpty()) {
- *errmsg = errobj.toString();
-
- // If every error has a code, and the code for all errors is the same, then add
- // a top-level field "code" with this value to the output object.
- if (commonErrCode > 0) {
- output->append("code", commonErrCode);
- output->append("codeName", ErrorCodes::errorString(ErrorCodes::Error(commonErrCode)));
- if (errors.size() == 1) {
- // Only propagate extra info if there was a single error object.
- if (auto extraInfo = errors.begin()->second.extraInfo()) {
- extraInfo->serialize(output);
- }
- }
+ for (const auto& error : nonIgnorableErrorsReceived) {
+ BSONObjBuilder statusObjBob;
+ CommandHelpers::appendCommandStatusNoThrow(statusObjBob, error.second);
+ appendRawResponse(error.first, statusObjBob.obj());
+ }
+ output->append("raw", rawShardResponses.done());
+
+ // If there were no non-ignorable errors, report success (possibly with a writeConcern error).
+ if (nonIgnorableErrorsReceived.empty()) {
+ if (firstWriteConcernErrorReceived) {
+ appendWriteConcernErrorToCmdResponse(firstWriteConcernErrorReceived->first,
+ firstWriteConcernErrorReceived->second,
+ *output);
}
- return false;
+ return true;
}
- return true;
+
+ // There was a non-ignorable error. Choose the first non-ignorable error as the top-level error.
+ const auto& firstNonIgnorableError = nonIgnorableErrorsReceived.front().second;
+ output->append("code", firstNonIgnorableError.code());
+ output->append("codeName", ErrorCodes::errorString(firstNonIgnorableError.code()));
+ *errmsg = firstNonIgnorableError.reason();
+ return false;
}
BSONObj extractQuery(const BSONObj& cmdObj) {
diff --git a/src/mongo/s/cluster_commands_helpers.h b/src/mongo/s/cluster_commands_helpers.h
index fce94d5fc43..1e3a9d91fbb 100644
--- a/src/mongo/s/cluster_commands_helpers.h
+++ b/src/mongo/s/cluster_commands_helpers.h
@@ -163,7 +163,7 @@ AsyncRequestsSender::Response executeCommandAgainstDatabasePrimary(
bool appendRawResponses(OperationContext* opCtx,
std::string* errmsg,
BSONObjBuilder* output,
- std::vector<AsyncRequestsSender::Response> shardResponses,
+ const std::vector<AsyncRequestsSender::Response>& shardResponses,
std::set<ErrorCodes::Error> ignoredErrors = {});
/**