diff options
author | Pavi Vetriselvan <pavithra.vetriselvan@mongodb.com> | 2020-11-17 15:35:15 -0500 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-03-17 01:12:55 +0000 |
commit | c8bca152e617ab9b7ec43f09aef0f0b40e439d4b (patch) | |
tree | c28a9c39865f0bec20d58af617cced0c65a39fe8 | |
parent | deb0d4434518321c65bb264558dd957c53cfd434 (diff) | |
download | mongo-c8bca152e617ab9b7ec43f09aef0f0b40e439d4b.tar.gz |
SERVER-50414 Reads should return not primary error messages during rollback if client sent helloOk
(cherry picked from commit d633efa72090a864442ef7232e1261d3b25bc27c)
(cherry picked from commit a04b42ed317f084387e70120fc0ad6ab0d6300ac)
6 files changed, 89 insertions, 6 deletions
diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml index 1c575e036c8..03e342a50ac 100644 --- a/etc/backports_required_for_multiversion_tests.yml +++ b/etc/backports_required_for_multiversion_tests.yml @@ -106,6 +106,8 @@ all: test_file: jstests/replsets/not_primary_errors_returned_if_client_sends_helloOk.js - ticket: SERVER-50412 test_file: jstests/sharding/mongos_helloOk_protocol.js + - ticket: SERVER-50414 + test_file: jstests/replsets/not_primary_errors_returned_during_rollback_if_helloOk.js # Tests that should only be excluded from particular suites should be listed under that suite. suites: diff --git a/jstests/concurrency/fsm_libs/cluster.js b/jstests/concurrency/fsm_libs/cluster.js index cc3dc0df1c5..be0daa47ff8 100644 --- a/jstests/concurrency/fsm_libs/cluster.js +++ b/jstests/concurrency/fsm_libs/cluster.js @@ -493,7 +493,7 @@ var Cluster = function(options) { res.databases.forEach(dbInfo => { // Don't perform listCollections on the admin or config database through a mongos // connection when stepping down the config server primary, because both are stored - // on the config server, and listCollections may return a not master error if the + // on the config server, and listCollections may return a NotPrimaryError if the // mongos is stale. // // TODO SERVER-30949: listCollections through mongos should automatically retry on diff --git a/jstests/replsets/not_primary_errors_returned_during_rollback_if_helloOk.js b/jstests/replsets/not_primary_errors_returned_during_rollback_if_helloOk.js new file mode 100644 index 00000000000..48fd2a88847 --- /dev/null +++ b/jstests/replsets/not_primary_errors_returned_during_rollback_if_helloOk.js @@ -0,0 +1,76 @@ +/* + * Tests that reads that fail during rollback with a NotPrimaryError will replace their + * "not master" error messages with "not primary" if the client sends "helloOk:true" as a part + * of their isMaster request. + * + * In practice, drivers will send "helloOk: true" in the initial handshake when + * opening a connection to the database. + * @tags: [requires_majority_read_concern] + */ + +(function() { +"use strict"; + +load("jstests/replsets/libs/rollback_test.js"); +load("jstests/replsets/rslib.js"); + +const dbName = "test"; +const collName = "not_primary_errors_returned_during_rollback_if_helloOk"; + +// Set up Rollback Test. +let rollbackTest = new RollbackTest(); + +// Insert a document to be read later. +assert.commandWorked(rollbackTest.getPrimary().getDB(dbName)[collName].insert({})); + +let rollbackNode = rollbackTest.transitionToRollbackOperations(); + +setFailPoint(rollbackNode, "rollbackHangAfterTransitionToRollback"); + +// Start rollback. +rollbackTest.transitionToSyncSourceOperationsBeforeRollback(); +rollbackTest.transitionToSyncSourceOperationsDuringRollback(); + +jsTestLog("Reconnecting to " + rollbackNode.host + " after rollback"); +reconnect(rollbackNode.getDB(dbName)); + +// Wait for rollback to hang. +checkLog.contains(rollbackNode, "rollbackHangAfterTransitionToRollback fail point enabled."); + +// Make sure we can't read during rollback. Since we want to exercise the real check for +// primary in the 'find' command, we have to disable the best-effort check for primary in service +// entry point. +setFailPoint(rollbackNode, "skipCheckingForNotPrimaryInCommandDispatch"); +jsTestLog("Reading during rollback returns not master error message"); +let res = assert.commandFailedWithCode(rollbackNode.getDB(dbName).runCommand({"find": collName}), + ErrorCodes.NotPrimaryOrSecondary, + "find did not fail with NotPrimaryOrSecondary"); + +// Since we did not send "helloOk: true", the error message should include "not master or +// secondary". +assert(res.errmsg.includes("not master or secondary"), res); + +// An isMaster response will not contain "helloOk: true" if the client does not send +// helloOk in the request. +res = assert.commandWorked(rollbackNode.getDB(dbName).adminCommand({isMaster: 1})); +assert.eq(res.helloOk, undefined); + +// Run the isMaster command with "helloOk: true" on the secondary. +res = assert.commandWorked(rollbackNode.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); + +jsTestLog("Reading during rollback returns not primary error message"); +res = assert.commandFailedWithCode(rollbackNode.getDB(dbName).runCommand({"find": collName}), + ErrorCodes.NotPrimaryOrSecondary, + "find did not fail with NotPrimaryOrSecondary"); +// Since we sent "helloOk: true", the error message should include "not primary or secondary". +assert(res.errmsg.includes("not primary or secondary"), res); +assert(!res.errmsg.includes("not master"), res); + +clearFailPoint(rollbackNode, "rollbackHangAfterTransitionToRollback"); + +rollbackTest.transitionToSteadyStateOperations(); +rollbackTest.stop(); +}()); diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index b355588bb0b..b9c730d12c1 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -2359,10 +2359,15 @@ Status ReplicationCoordinatorImpl::checkCanServeReadsFor_UNSAFE(OperationContext if (isPrimaryOrSecondary) { return Status::OK(); } - return Status(ErrorCodes::NotPrimaryOrSecondary, - "not master or secondary; cannot currently read from this replSet member"); + const auto msg = client->supportsHello() + ? "not primary or secondary; cannot currently read from this replSet member"_sd + : "not master or secondary; cannot currently read from this replSet member"_sd; + return Status(ErrorCodes::NotPrimaryOrSecondary, msg); } - return Status(ErrorCodes::NotPrimaryNoSecondaryOk, "not master and slaveOk=false"); + + const auto msg = client->supportsHello() ? "not primary and secondaryOk=false"_sd + : "not master and slaveOk=false"_sd; + return Status(ErrorCodes::NotPrimaryNoSecondaryOk, msg); } bool ReplicationCoordinatorImpl::isInPrimaryOrSecondaryState(OperationContext* opCtx) const { diff --git a/src/mongo/db/repl/replication_coordinator_impl_test.cpp b/src/mongo/db/repl/replication_coordinator_impl_test.cpp index 42433a2756b..a072a500a08 100644 --- a/src/mongo/db/repl/replication_coordinator_impl_test.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl_test.cpp @@ -2603,7 +2603,7 @@ TEST_F(StepDownTest, InterruptingAfterUnconditionalStepdownDoesNotRestoreWriteAv stepDownStatus == ErrorCodes::Interrupted); ASSERT_TRUE(getReplCoord()->getMemberState().secondary()); - // We should still be indicating that we are not master. + // We should still be indicating that we are not a writable primary. getReplCoord()->fillIsMasterForReplSet(&response, {}); ASSERT_FALSE(response.isMaster()); diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h index 21f6c7a89db..3c11e41c523 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.h +++ b/src/mongo/db/repl/replication_coordinator_mock.h @@ -296,7 +296,7 @@ public: AwaitReplicationReturnValueFunction returnValueFunction); /** - * Always allow writes even if this node is not master. Used by sharding unit tests. + * Always allow writes even if this node is a writable primary. Used by sharding unit tests. */ void alwaysAllowWrites(bool allowWrites); |