summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPavi Vetriselvan <pavithra.vetriselvan@mongodb.com>2020-10-20 19:32:23 -0400
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2021-03-17 01:12:55 +0000
commit0aa0aad87fdd50f98abe0640385ab5a2bc443928 (patch)
treed55769614ee03a57c13ba5780b20e7763a380080
parent2a1fc45cc1d106be07da66300782420f8011ac1e (diff)
downloadmongo-0aa0aad87fdd50f98abe0640385ab5a2bc443928.tar.gz
SERVER-50412 implement helloOk protocol negotiation on mongod
(cherry picked from commit 1118ad680f76ac29795c8396beb11b0b50322f20) (cherry picked from commit 4dbefcfc9304a9ffa1fec8390dc2bd30e0dc9a54)
-rw-r--r--etc/backports_required_for_multiversion_tests.yml2
-rw-r--r--jstests/replsets/not_primary_errors_returned_if_client_sends_helloOk.js77
-rw-r--r--src/mongo/db/client.h20
-rw-r--r--src/mongo/db/repl/replication_info.cpp18
-rw-r--r--src/mongo/db/service_entry_point_common.cpp11
5 files changed, 125 insertions, 3 deletions
diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 47ddd16346d..bd64ec3b1e2 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -102,6 +102,8 @@ all:
test_file: jstests/replsets/force_shutdown_primary.js
- ticket: SERVER-52953
test_file: jstests/core/geo_near_point_query.js
+ - ticket: SERVER-50412
+ test_file: jstests/replsets/not_primary_errors_returned_if_client_sends_helloOk.js
# Tests that should only be excluded from particular suites should be listed under that suite.
suites:
diff --git a/jstests/replsets/not_primary_errors_returned_if_client_sends_helloOk.js b/jstests/replsets/not_primary_errors_returned_if_client_sends_helloOk.js
new file mode 100644
index 00000000000..e8cca15064a
--- /dev/null
+++ b/jstests/replsets/not_primary_errors_returned_if_client_sends_helloOk.js
@@ -0,0 +1,77 @@
+/*
+ * Tests that if a client sends "helloOk: true" as a part of their isMaster request,
+ * mongod will replace "not master" error messages with "not primary".
+ *
+ * In practice, drivers will send "helloOk: true" in the initial handshake when
+ * opening a connection to the database.
+ */
+
+(function() {
+"use strict";
+
+const dbName = "test";
+const collName = "not_primary_errors_returned_if_client_sends_helloOk";
+const rst = new ReplSetTest({nodes: 2});
+rst.startSet();
+rst.initiate();
+const primary = rst.getPrimary();
+const secondary = rst.getSecondary();
+
+assert.commandWorked(primary.getDB(dbName)[collName].insert({x: 1}));
+rst.awaitReplication();
+
+// Attempts to write to a secondary should fail with NotWritablePrimary.
+let res = assert.commandFailedWithCode(secondary.getDB(dbName)[collName].insert({y: 2}),
+ ErrorCodes.NotWritablePrimary,
+ "insert did not fail with NotWritablePrimary");
+// Since the shell opens connections without using "helloOk: true", the error message
+// should include "not master".
+assert(res.errmsg.includes("not master"), res);
+
+// Set secondaryOk to false, disallowing reads on secondaries.
+secondary.getDB(dbName).getMongo().setSecondaryOk(false);
+assert.eq(secondary.getDB(dbName).getMongo().getSecondaryOk(), false);
+res = assert.commandFailedWithCode(secondary.getDB(dbName).runCommand({find: collName}),
+ ErrorCodes.NotPrimaryNoSecondaryOk,
+ "find did not fail with NotPrimaryNoSecondaryOk");
+// Since we did not send "helloOk: true", the error message should include "not master".
+assert(res.errmsg.includes("not master"), res);
+
+// An isMaster response will not contain "helloOk: true" if the client does not send
+// helloOk in the request.
+res = assert.commandWorked(secondary.getDB(dbName).adminCommand({isMaster: 1}));
+assert.eq(res.helloOk, undefined);
+
+try {
+ // "helloOk" field type must be a boolean.
+ secondary.getDB(dbName).adminCommand({isMaster: 1, helloOk: 3});
+} catch (e) {
+ assert.eq(e.code, ErrorCodes.TypeMismatch, "invalid helloOk field did not fail to parse");
+}
+
+// Run the isMaster command with "helloOk: true" on the secondary.
+res = assert.commandWorked(secondary.getDB(dbName).adminCommand({isMaster: 1, helloOk: true}));
+// The response should contain "helloOk: true", which indicates to the client that the
+// server supports the hello command.
+assert.eq(res.helloOk, true);
+
+// Attempts to write to a secondary should fail with NotWritablePrimary.
+res = assert.commandFailedWithCode(secondary.getDB(dbName)[collName].insert({z: 3}),
+ ErrorCodes.NotWritablePrimary,
+ "insert did not fail with NotWritablePrimary");
+// Since we sent "helloOk: true", the error message should include "not primary".
+assert(res.errmsg.includes("not primary"), res);
+assert(!res.errmsg.includes("not master"), res);
+
+// secondaryOk was already set to false, so the following read should still fail with
+// NotPrimaryNoSecondaryOk.
+assert.eq(secondary.getDB(dbName).getMongo().getSecondaryOk(), false);
+res = assert.commandFailedWithCode(secondary.getDB(dbName).runCommand({find: collName}),
+ ErrorCodes.NotPrimaryNoSecondaryOk,
+ "find did not fail with NotPrimaryNoSecondaryOk");
+// Since we sent "helloOk: true", the error message should include "not primary".
+assert(res.errmsg.includes("not primary"), res);
+assert(!res.errmsg.includes("not master"), res);
+
+rst.stopSet();
+})(); \ No newline at end of file
diff --git a/src/mongo/db/client.h b/src/mongo/db/client.h
index 2c06e0e1453..7d63bb39b9f 100644
--- a/src/mongo/db/client.h
+++ b/src/mongo/db/client.h
@@ -243,6 +243,22 @@ public:
return _killed.loadRelaxed();
}
+ /**
+ * Whether this client supports the hello command, which indicates that the server
+ * can return "not primary" error messages.
+ */
+ bool supportsHello() const {
+ return _supportsHello;
+ }
+
+ /**
+ * Will be set to true if the client sent { helloOk: true } when opening a
+ * connection to the server. Defaults to false.
+ */
+ void setSupportsHello(bool newVal) {
+ _supportsHello = newVal;
+ }
+
private:
friend class ServiceContext;
friend class ThreadClient;
@@ -274,6 +290,10 @@ private:
PseudoRandom _prng;
AtomicWord<bool> _killed{false};
+
+ // Whether this client used { helloOk: true } when opening its connection, indicating that
+ // it supports the hello command.
+ bool _supportsHello = false;
};
/**
diff --git a/src/mongo/db/repl/replication_info.cpp b/src/mongo/db/repl/replication_info.cpp
index 594baf38b59..2446336e382 100644
--- a/src/mongo/db/repl/replication_info.cpp
+++ b/src/mongo/db/repl/replication_info.cpp
@@ -34,6 +34,7 @@
#include <vector>
#include "mongo/base/string_data.h"
+#include "mongo/bson/util/bson_extract.h"
#include "mongo/client/connpool.h"
#include "mongo/client/dbclient_connection.h"
#include "mongo/db/auth/sasl_mechanism_registry.h"
@@ -370,6 +371,23 @@ public:
appendReplicationInfo(opCtx, result, 0, useLegacyResponseFields());
+ // Try to parse the optional 'helloOk' field. This should be provided on the initial
+ // handshake for an incoming connection if the client supports the hello command. Clients
+ // that specify 'helloOk' do not rely on "not master" error message parsing, which means
+ // that we can safely return "not primary" error messages instead.
+ bool helloOk = opCtx->getClient()->supportsHello();
+ Status status = bsonExtractBooleanField(cmdObj, "helloOk", &helloOk);
+ if (status.isOK()) {
+ // If the hello request contains a "helloOk" field, set _supportsHello on the Client
+ // to the value.
+ opCtx->getClient()->setSupportsHello(helloOk);
+ // Attach helloOk: true to the response so that the client knows the server supports
+ // the hello command.
+ result.append("helloOk", true);
+ } else if (status.code() != ErrorCodes::NoSuchKey) {
+ uassertStatusOK(status);
+ }
+
if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) {
const int configServerModeNumber = 2;
result.append("configsvr", configServerModeNumber);
diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp
index 9a4b200d37a..e6997ed7877 100644
--- a/src/mongo/db/service_entry_point_common.cpp
+++ b/src/mongo/db/service_entry_point_common.cpp
@@ -775,13 +775,18 @@ void execCommandDatabase(OperationContext* opCtx,
couldHaveOptedIn && ReadPreferenceSetting::get(opCtx).canRunOnSecondary();
bool canRunHere = commandCanRunHere(opCtx, dbname, command, inMultiDocumentTransaction);
if (!canRunHere && couldHaveOptedIn) {
- uasserted(ErrorCodes::NotPrimaryNoSecondaryOk, "not master and slaveOk=false");
+ const auto msg = opCtx->getClient()->supportsHello()
+ ? "not primary and secondaryOk=false"_sd
+ : "not master and slaveOk=false"_sd;
+ uasserted(ErrorCodes::NotPrimaryNoSecondaryOk, msg);
}
if (MONGO_FAIL_POINT(respondWithNotPrimaryInCommandDispatch)) {
uassert(ErrorCodes::NotWritablePrimary, "not primary", canRunHere);
} else {
- uassert(ErrorCodes::NotWritablePrimary, "not master", canRunHere);
+ const auto msg =
+ opCtx->getClient()->supportsHello() ? "not primary"_sd : "not master"_sd;
+ uassert(ErrorCodes::NotWritablePrimary, msg, canRunHere);
}
if (!command->maintenanceOk() &&
@@ -1074,7 +1079,7 @@ DbResponse receivedCommands(OperationContext* opCtx,
notPrimaryUnackWrites.increment();
uasserted(ErrorCodes::NotWritablePrimary,
str::stream()
- << "Not-master error while processing '" << request.getCommandName()
+ << "Not-primary error while processing '" << request.getCommandName()
<< "' operation on '" << request.getDatabase() << "' database via "
<< "fire-and-forget command execution.");
}