From 8096dda89619b3643234b62a48efd35bb335ee43 Mon Sep 17 00:00:00 2001 From: Tess Avitabile Date: Fri, 17 Jan 2020 15:39:42 +0000 Subject: SERVER-44520 Make mongo's isMaster wait maxAwaitTimeMS --- jstests/noPassthrough/awaitable_ismaster.js | 160 +++++++++++++++++++++++++ jstests/replsets/awaitable_ismaster.js | 126 ------------------- src/mongo/s/commands/cluster_is_master_cmd.cpp | 52 ++++++++ 3 files changed, 212 insertions(+), 126 deletions(-) create mode 100644 jstests/noPassthrough/awaitable_ismaster.js delete mode 100644 jstests/replsets/awaitable_ismaster.js diff --git a/jstests/noPassthrough/awaitable_ismaster.js b/jstests/noPassthrough/awaitable_ismaster.js new file mode 100644 index 00000000000..3899f7a8f4c --- /dev/null +++ b/jstests/noPassthrough/awaitable_ismaster.js @@ -0,0 +1,160 @@ +/** + * Tests the maxAwaitTimeMS and topologyVersion parameters of the isMaster command. + * @tags: [requires_replication] + */ +(function() { +"use strict"; +load("jstests/libs/parallel_shell_helpers.js"); + +function runTest(db) { + // Check isMaster response contains a topologyVersion even if maxAwaitTimeMS and topologyVersion + // are not included in the request. + const res = assert.commandWorked(db.runCommand({isMaster: 1})); + assert(res.hasOwnProperty("topologyVersion"), tojson(res)); + + const topologyVersionField = res.topologyVersion; + assert(topologyVersionField.hasOwnProperty("processId"), tojson(topologyVersionField)); + assert(topologyVersionField.hasOwnProperty("counter"), tojson(topologyVersionField)); + + // Check that isMaster succeeds when passed a valid topologyVersion and maxAwaitTimeMS. In this + // case, use the topologyVersion from the previous isMaster response. The topologyVersion field + // is expected to be of the form {processId: , counter: }. + assert.commandWorked( + db.runCommand({isMaster: 1, topologyVersion: topologyVersionField, maxAwaitTimeMS: 0})); + + // Ensure isMaster waits for at least maxAwaitTimeMS before returning. + let now = new Date(); + assert.commandWorked( + db.runCommand({isMaster: 1, topologyVersion: topologyVersionField, maxAwaitTimeMS: 2000})); + let isMasterDuration = new Date() - now; + // Allow for some clock imprecision between the server and the jstest. + assert.gte( + isMasterDuration, + 1000, + `isMaster should have taken at least 1000ms, but completed in ${isMasterDuration}ms`); + + // Check that when a different processId is given, the server responds immediately. + now = new Date(); + assert.commandWorked(db.runCommand({ + isMaster: 1, + topologyVersion: {processId: ObjectId(), counter: topologyVersionField.counter}, + maxAwaitTimeMS: 2000 + })); + isMasterDuration = new Date() - now; + assert.lt(isMasterDuration, + 1000, + `isMaster should have taken at most 1000ms, but completed in ${isMasterDuration}ms`); + + // Check that when a different processId is given, a higher counter is allowed. + assert.commandWorked(db.runCommand({ + isMaster: 1, + topologyVersion: + {processId: ObjectId(), counter: NumberLong(topologyVersionField.counter + 1)}, + maxAwaitTimeMS: 0 + })); + + // Check that when the same processId is given, a higher counter is not allowed. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: { + processId: topologyVersionField.processId, + counter: NumberLong(topologyVersionField.counter + 1) + }, + maxAwaitTimeMS: 0 + }), + [31382, 51761]); + + // Check that passing a topologyVersion not of type object fails. + assert.commandFailedWithCode( + db.runCommand({isMaster: 1, topologyVersion: "topology_version_string", maxAwaitTimeMS: 0}), + 10065); + + // Check that a topologyVersion with an invalid processId and valid counter fails. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: {processId: "pid1", counter: topologyVersionField.counter}, + maxAwaitTimeMS: 0 + }), + ErrorCodes.TypeMismatch); + + // Check that a topologyVersion with a valid processId and invalid counter fails. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: {processId: topologyVersionField.processId, counter: 0}, + maxAwaitTimeMS: 0 + }), + ErrorCodes.TypeMismatch); + + // Check that a topologyVersion with a valid processId but missing counter fails. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: {processId: topologyVersionField.processId}, + maxAwaitTimeMS: 0 + }), + 40414); + + // Check that a topologyVersion with a missing processId and valid counter fails. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: {counter: topologyVersionField.counter}, + maxAwaitTimeMS: 0 + }), + 40414); + + // Check that a topologyVersion with a valid processId and negative counter fails. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: {processId: topologyVersionField.processId, counter: NumberLong("-1")}, + maxAwaitTimeMS: 0 + }), + [31372, 51758]); + + // Check that isMaster fails if there is an extra field in its topologyVersion. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: { + processId: topologyVersionField.processId, + counter: topologyVersionField.counter, + randomField: "I should cause an error" + }, + maxAwaitTimeMS: 0 + }), + 40415); + + // A client following the awaitable isMaster protocol must include topologyVersion in their + // request if and only if they include maxAwaitTimeMS. Check that isMaster fails if there is a + // topologyVersion but no maxAwaitTimeMS field. + assert.commandFailedWithCode( + db.runCommand({isMaster: 1, topologyVersion: topologyVersionField}), [31368, 51760]); + + // Check that isMaster fails if there is a maxAwaitTimeMS field but no topologyVersion. + assert.commandFailedWithCode(db.runCommand({isMaster: 1, maxAwaitTimeMS: 0}), [31368, 51760]); + + // Check that isMaster fails if there is a valid topologyVersion but invalid maxAwaitTimeMS + // type. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: topologyVersionField, + maxAwaitTimeMS: "stringMaxAwaitTimeMS" + }), + ErrorCodes.TypeMismatch); + + // Check that isMaster fails if there is a valid topologyVersion but negative maxAwaitTimeMS. + assert.commandFailedWithCode(db.runCommand({ + isMaster: 1, + topologyVersion: topologyVersionField, + maxAwaitTimeMS: -1, + }), + [31373, 51759]); +} + +const replTest = new ReplSetTest({nodes: 1}); +replTest.startSet(); +replTest.initiate(); +runTest(replTest.getPrimary().getDB("admin")); +replTest.stopSet(); + +const st = new ShardingTest({mongos: 1, shards: [{nodes: 1}], config: 1}); +runTest(st.s.getDB("admin")); +st.stop(); +})(); diff --git a/jstests/replsets/awaitable_ismaster.js b/jstests/replsets/awaitable_ismaster.js deleted file mode 100644 index 9887038f41d..00000000000 --- a/jstests/replsets/awaitable_ismaster.js +++ /dev/null @@ -1,126 +0,0 @@ -/** - * Tests the maxAwaitTimeMS and topologyVersion parameters of the isMaster command. - * @tags: [requires_fcv_44] - */ -(function() { -"use strict"; -load("jstests/libs/parallel_shell_helpers.js"); - -// Test isMaster paramaters on a single node replica set. -const replSetName = "awaitable_ismaster_test"; -const replTest = new ReplSetTest({name: replSetName, nodes: 1}); -replTest.startSet(); -replTest.initiate(); - -const dbName = "awaitable_ismaster_test"; -const node = replTest.getPrimary(); -const db = node.getDB(dbName); - -// Check isMaster response contains a topologyVersion even if maxAwaitTimeMS and topologyVersion are -// not included in the request. -const res = assert.commandWorked(db.runCommand({isMaster: 1})); -assert(res.hasOwnProperty("topologyVersion"), tojson(res)); - -const topologyVersionField = res.topologyVersion; -assert(topologyVersionField.hasOwnProperty("processId"), tojson(topologyVersionField)); -assert(topologyVersionField.hasOwnProperty("counter"), tojson(topologyVersionField)); - -// Check that isMaster succeeds when passed a valid toplogyVersion and maxAwaitTimeMS. In this case, -// use the topologyVersion from the previous isMaster response. The topologyVersion field is -// expected to be of the form {processId: , counter: }. -assert.commandWorked( - db.runCommand({isMaster: 1, topologyVersion: topologyVersionField, maxAwaitTimeMS: 0})); - -// Ensure isMaster waits for at least maxAwaitTimeMS before returning. -const now = new Date(); -assert.commandWorked( - db.runCommand({isMaster: 1, topologyVersion: topologyVersionField, maxAwaitTimeMS: 2000})); -const isMasterDuration = new Date() - now; - -// Allow for some clock imprecision between the server and the jstest. -assert.gte(isMasterDuration, - 1000, - `isMaster should have taken at least 1000ms, but completed in ${isMasterDuration}ms`); - -// Check that passing a topologyVersion not of type object fails. -assert.commandFailedWithCode( - db.runCommand({isMaster: 1, topologyVersion: "topology_version_string", maxAwaitTimeMS: 0}), - 10065); - -// Check that a topologyVersion with an invalid processId and valid counter fails. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: {processId: "pid1", counter: topologyVersionField.counter}, - maxAwaitTimeMS: 0 -}), - ErrorCodes.TypeMismatch); - -// Check that a topologyVersion with a valid processId and invalid counter fails. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: {processId: topologyVersionField.processId, counter: 0}, - maxAwaitTimeMS: 0 -}), - ErrorCodes.TypeMismatch); - -// Check that a topologyVersion with a valid processId but missing counter fails. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: {processId: topologyVersionField.processId}, - maxAwaitTimeMS: 0 -}), - 40414); - -// Check that a topologyVersion with a missing processId and valid counter fails. -assert.commandFailedWithCode( - db.runCommand( - {isMaster: 1, topologyVersion: {counter: topologyVersionField.counter}, maxAwaitTimeMS: 0}), - 40414); - -// Check that a topologyVersion with a valid processId and negative counter fails. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: {processId: topologyVersionField.processId, counter: NumberLong("-1")}, - maxAwaitTimeMS: 0 -}), - 31372); - -// Check that isMaster fails if there is an extra field in its topologyVersion. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: { - processId: topologyVersionField.processId, - counter: topologyVersionField.counter, - randomField: "I should cause an error" - }, - maxAwaitTimeMS: 0 -}), - 40415); - -// A client following the awaitable isMaster protocol must include topologyVersion in their request -// if and only if they include maxAwaitTimeMS. -// Check that isMaster fails if there is a topologyVersion but no maxAwaitTimeMS field. -assert.commandFailedWithCode(db.runCommand({isMaster: 1, topologyVersion: topologyVersionField}), - 31368); - -// Check that isMaster fails if there is a maxAwaitTimeMS field but no topologyVersion. -assert.commandFailedWithCode(db.runCommand({isMaster: 1, maxAwaitTimeMS: 0}), 31368); - -// Check that isMaster fails if there is a valid topologyVersion but invalid maxAwaitTimeMS type. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: topologyVersionField, - maxAwaitTimeMS: "stringMaxAwaitTimeMS" -}), - ErrorCodes.TypeMismatch); - -// Check that isMaster fails if there is a valid topologyVersion but negative maxAwaitTimeMS. -assert.commandFailedWithCode(db.runCommand({ - isMaster: 1, - topologyVersion: topologyVersionField, - maxAwaitTimeMS: -1, -}), - 31373); - -replTest.stopSet(); -})(); diff --git a/src/mongo/s/commands/cluster_is_master_cmd.cpp b/src/mongo/s/commands/cluster_is_master_cmd.cpp index af00e62bd09..ccd0d7d00d5 100644 --- a/src/mongo/s/commands/cluster_is_master_cmd.cpp +++ b/src/mongo/s/commands/cluster_is_master_cmd.cpp @@ -26,9 +26,11 @@ * 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::kCommand #include "mongo/platform/basic.h" +#include "mongo/bson/util/bson_extract.h" #include "mongo/db/auth/sasl_mechanism_registry.h" #include "mongo/db/client.h" #include "mongo/db/commands.h" @@ -38,7 +40,9 @@ #include "mongo/db/wire_version.h" #include "mongo/rpc/metadata/client_metadata.h" #include "mongo/rpc/metadata/client_metadata_ismaster.h" +#include "mongo/rpc/topology_version_gen.h" #include "mongo/transport/message_compressor_manager.h" +#include "mongo/util/log.h" #include "mongo/util/map_util.h" #include "mongo/util/net/socket_utils.h" #include "mongo/util/version.h" @@ -47,6 +51,13 @@ namespace mongo { MONGO_FAIL_POINT_DEFINE(waitInIsMaster); +TopologyVersion mongosTopologyVersion; + +MONGO_INITIALIZER(GenerateMongosTopologyVersion)(InitializerContext*) { + mongosTopologyVersion = TopologyVersion(OID::gen(), 0); + return Status::OK(); +} + namespace { class CmdIsMaster : public BasicCommand { @@ -112,6 +123,44 @@ public: opCtx->getClient(), std::move(swParseClientMetadata.getValue())); } + // If a client is following the awaitable isMaster protocol, maxAwaitTimeMS should be + // present if and only if topologyVersion is present in the request. + auto topologyVersionElement = cmdObj["topologyVersion"]; + auto maxAwaitTimeMSField = cmdObj["maxAwaitTimeMS"]; + if (topologyVersionElement && maxAwaitTimeMSField) { + auto clientTopologyVersion = TopologyVersion::parse( + IDLParserErrorContext("TopologyVersion"), topologyVersionElement.Obj()); + uassert(51758, + "topologyVersion must have a non-negative counter", + clientTopologyVersion.getCounter() >= 0); + + long long maxAwaitTimeMS; + uassertStatusOK(bsonExtractIntegerField(cmdObj, "maxAwaitTimeMS", &maxAwaitTimeMS)); + uassert(51759, "maxAwaitTimeMS must be a non-negative integer", maxAwaitTimeMS >= 0); + + LOG(3) << "Using maxAwaitTimeMS for awaitable isMaster protocol."; + + if (clientTopologyVersion.getProcessId() == mongosTopologyVersion.getProcessId()) { + uassert(51761, + str::stream() + << "Received a topology version with counter: " + << clientTopologyVersion.getCounter() + << " which is greater than the mongos topology version counter: " + << mongosTopologyVersion.getCounter(), + clientTopologyVersion.getCounter() == mongosTopologyVersion.getCounter()); + + // The topologyVersion never changes on a running mongos process, so just sleep for + // maxAwaitTimeMS. + opCtx->sleepFor(Milliseconds(maxAwaitTimeMS)); + } + } else { + uassert(51760, + (topologyVersionElement + ? "A request with a 'topologyVersion' must include 'maxAwaitTimeMS'" + : "A request with 'maxAwaitTimeMS' must include a 'topologyVersion'"), + !topologyVersionElement && !maxAwaitTimeMSField); + } + result.appendBool("ismaster", true); result.append("msg", "isdbgrid"); result.appendNumber("maxBsonObjectSize", BSONObjMaxUserSize); @@ -138,6 +187,9 @@ public: auto& saslMechanismRegistry = SASLServerMechanismRegistry::get(opCtx->getServiceContext()); saslMechanismRegistry.advertiseMechanismNamesForUser(opCtx, cmdObj, &result); + BSONObjBuilder topologyVersionBuilder(result.subobjStart("topologyVersion")); + mongosTopologyVersion.serialize(&topologyVersionBuilder); + return true; } -- cgit v1.2.1