diff options
author | Ali Mir <ali.mir@mongodb.com> | 2020-08-06 13:44:25 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-09-16 01:41:14 +0000 |
commit | 48f1cdf6c9cff10639c4a8eeebe72394a4f5cabb (patch) | |
tree | ba8346420139a12051fad9bb4b201f2fa8a9da22 | |
parent | c18aec098f8d604b5cce4a9c8bc14e37b53a4a25 (diff) | |
download | mongo-48f1cdf6c9cff10639c4a8eeebe72394a4f5cabb.tar.gz |
SERVER-49987 Rename response fields if hello command is sent on mongod
(cherry picked from commit e9d2fbd27574714fc3c2db7cd45d413b7fbd4718)
(cherry picked from commit 7157e90233ddc3b1b208564a711289a2d99e5ee5)
-rw-r--r-- | jstests/core/ismaster.js | 20 | ||||
-rw-r--r-- | jstests/replsets/ismaster1.js | 198 | ||||
-rw-r--r-- | src/mongo/db/repl/is_master_response.cpp | 30 | ||||
-rw-r--r-- | src/mongo/db/repl/is_master_response.h | 16 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_info.cpp | 20 |
5 files changed, 198 insertions, 86 deletions
diff --git a/jstests/core/ismaster.js b/jstests/core/ismaster.js index 4f2b41c64f7..e2cd642c61a 100644 --- a/jstests/core/ismaster.js +++ b/jstests/core/ismaster.js @@ -14,11 +14,16 @@ function checkResponseFields(commandString) { "maxMessageSizeBytes possibly missing:" + tojson(res)); assert(res.maxWriteBatchSize && isNumber(res.maxWriteBatchSize) && res.maxWriteBatchSize > 0, "maxWriteBatchSize possibly missing:" + tojson(res)); - assert.eq("boolean", typeof res.ismaster, "ismaster field is not a boolean" + tojson(res)); - // TODO SERVER-49987: Check for res.isWritablePrimary instead of res.ismaster if a hello command - // was executed. - assert(res.ismaster === true, "ismaster field is false" + tojson(res)); + if (commandString === "hello") { + assert.eq("boolean", + typeof res.isWritablePrimary, + "isWritablePrimary field is not a boolean" + tojson(res)); + assert(res.isWritablePrimary === true, "isWritablePrimary field is false" + tojson(res)); + } else { + assert.eq("boolean", typeof res.ismaster, "ismaster field is not a boolean" + tojson(res)); + assert(res.ismaster === true, "ismaster field is false" + tojson(res)); + } assert(res.localTime, "localTime possibly missing:" + tojson(res)); if (!testingReplication) { @@ -31,9 +36,10 @@ function checkResponseFields(commandString) { "passives", "arbiters", "primary", - "aribterOnly", + "arbiterOnly", "passive", "slaveDelay", + "secondaryDelaySecs", "hidden", "tags", "buildIndexes", @@ -55,8 +61,6 @@ function checkResponseFields(commandString) { } } -// TODO SERVER-49987: add the "hello" command test back in once mongod response fields are -// appropriately changed. -// checkResponseFields("hello"); +checkResponseFields("hello"); checkResponseFields("ismaster"); checkResponseFields("isMaster"); diff --git a/jstests/replsets/ismaster1.js b/jstests/replsets/ismaster1.js index cfb50a12912..3fdcce6463e 100644 --- a/jstests/replsets/ismaster1.js +++ b/jstests/replsets/ismaster1.js @@ -1,6 +1,7 @@ /** - * check all the easily testable fields in the document returned by the isMaster() command - * also checks that fields that should not be in the document are absent + * Checks all the easily testable fields in the response object returned by the hello() command and + * its aliases, isMaster() and ismaster(). This test also checks that fields that should not be in + * the document are absent. */ // Skip db hash check because node 2 is slave delayed and may time out on awaitReplication. @@ -28,10 +29,36 @@ var generateErrorString = function(badFields, missingFields, badValues, result) return str; }; -// function to check a single result -var checkMember = function(memberInfo) { - // run isMaster on the connection - result = memberInfo.conn.getDB("admin").runCommand({isMaster: 1}); +// This function calls checkResponseFields with the isMaster and hello commands. +var runHelloCmdAndAliases = function(memberInfo) { + checkResponseFields(memberInfo, "ismaster"); + checkResponseFields(memberInfo, "isMaster"); + checkResponseFields(memberInfo, "hello"); +}; + +// This function runs either the isMaster or hello command, and validates that the response is what +// we expect. +var checkResponseFields = function(memberInfo, cmd) { + // run the passed in command on the connection + var result = memberInfo.conn.getDB("admin").runCommand(cmd); + // If we are running the hello command, we must modify the expected fields. We expect + // "isWritablePrimary" and "secondaryDelaySecs" instead of "ismaster" and "slaveDelay" in the + // hello command response. + if (cmd === "hello") { + memberInfo.goodValues.isWritablePrimary = memberInfo.goodValues.ismaster; + delete memberInfo.goodValues.ismaster; + memberInfo.unwantedFields.push("ismaster"); + memberInfo.unwantedFields = + memberInfo.unwantedFields.filter(f => f !== "isWritablePrimary"); + + if (memberInfo.goodValues.hasOwnProperty("slaveDelay")) { + memberInfo.goodValues.secondaryDelaySecs = memberInfo.goodValues.slaveDelay; + delete memberInfo.goodValues.slaveDelay; + memberInfo.unwantedFields.push("slaveDelay"); + memberInfo.unwantedFields = + memberInfo.unwantedFields.filter(f => f !== "secondaryDelaySecs"); + } + } // make sure result doesn't contain anything it shouldn't var badFields = []; @@ -78,7 +105,7 @@ var checkMember = function(memberInfo) { }; // start of test code -var name = "ismaster"; +var name = "hello_and_aliases"; var replTest = new ReplSetTest({name: name, nodes: 4}); var nodes = replTest.startSet(); @@ -96,54 +123,68 @@ var agreeOnPrimaryAndSetVersion = function(setVersion) { print("Waiting for primary and replica set version " + setVersion); var nodes = replTest.nodes; - var primary = undefined; + var currPrimary = undefined; var lastSetVersion = setVersion; for (var i = 0; i < nodes.length; i++) { try { - var isMasterResult = nodes[i].getDB("admin").runCommand({isMaster: 1}); + var helloResult = nodes[i].getDB("admin").runCommand({hello: 1}); } catch (e) { // handle reconnect errors due to step downs - print("Error while calling isMaster on " + nodes[i] + ": " + e); + print("Error while calling hello on " + nodes[i] + ": " + e); return false; } - printjson(isMasterResult); - if (!primary) - primary = isMasterResult.primary; + printjson(helloResult); + if (!currPrimary) + currPrimary = helloResult.primary; if (!lastSetVersion) - lastSetVersion = isMasterResult.setVersion; - if (isMasterResult.primary != primary || !primary) + lastSetVersion = helloResult.setVersion; + if (helloResult.primary != currPrimary || !currPrimary) return false; - if (isMasterResult.setVersion != lastSetVersion) + if (helloResult.setVersion != lastSetVersion) return false; } return true; }; -var master = replTest.getPrimary(); +var primary = replTest.getPrimary(); var expectedVersion = replTest.getReplSetConfigFromNode().version; assert.soon(function() { return agreeOnPrimaryAndSetVersion(expectedVersion); }, "Nodes did not initiate in less than a minute", 60000); -// check to see if the information from isMaster() is correct at each node -// the checker only checks that the field exists when its value is "has" -checkMember({ - conn: master, - name: "master", - goodValues: - {setName: "ismaster", setVersion: expectedVersion, ismaster: true, secondary: false, ok: 1}, +// Check to see if the information from hello() and its aliases are correct at each node. +// The checker only checks that the field exists when its value is "has". +runHelloCmdAndAliases({ + conn: primary, + name: "primary", + goodValues: { + setName: "hello_and_aliases", + setVersion: expectedVersion, + ismaster: true, + secondary: false, + ok: 1 + }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["arbiterOnly", "passive", "slaveDelay", "hidden", "tags", "buildIndexes"] + unwantedFields: [ + "isWritablePrimary", + "arbiterOnly", + "passive", + "slaveDelay", + "secondaryDelaySecs", + "hidden", + "tags", + "buildIndexes" + ] }); -checkMember({ +runHelloCmdAndAliases({ conn: replTest.liveNodes.slaves[0], - name: "slave", + name: "secondary", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -152,14 +193,22 @@ checkMember({ }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["arbiterOnly", "slaveDelay", "hidden", "tags", "buildIndexes"] + unwantedFields: [ + "isWritablePrimary", + "arbiterOnly", + "slaveDelay", + "secondaryDelaySecs", + "hidden", + "tags", + "buildIndexes" + ] }); -checkMember({ +runHelloCmdAndAliases({ conn: replTest.liveNodes.slaves[1], - name: "delayed_slave", + name: "delayed_secondary", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -170,14 +219,14 @@ checkMember({ }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["arbiterOnly", "tags"] + unwantedFields: ["isWritablePrimary", "arbiterOnly", "tags", "secondaryDelaySecs"] }); -checkMember({ +runHelloCmdAndAliases({ conn: replTest.liveNodes.slaves[2], name: "arbiter", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: false, secondary: false, @@ -186,11 +235,19 @@ checkMember({ }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["slaveDelay", "hidden", "tags", "buildIndexes", "passive"] + unwantedFields: [ + "isWritablePrimary", + "slaveDelay", + "secondaryDelaySecs", + "hidden", + "tags", + "buildIndexes", + "passive" + ] }); -// reconfigure and make sure the changes show up in ismaster on all members -config = master.getDB("local").system.replset.findOne(); +// Reconfigure the replset and make sure the changes are present on all members. +config = primary.getDB("local").system.replset.findOne(); config.version = config.version + 1; config.members[0].tags = { disk: "ssd" @@ -204,23 +261,23 @@ config.members[2].tags = { disk: "hdd" }; try { - result = master.getDB("admin").runCommand({replSetReconfig: config}); + result = primary.getDB("admin").runCommand({replSetReconfig: config}); } catch (e) { print(e); } -master = replTest.getPrimary(); +primary = replTest.getPrimary(); expectedVersion = config.version; assert.soon(function() { return agreeOnPrimaryAndSetVersion(expectedVersion); }, "Nodes did not sync in less than a minute", 60000); // check nodes for their new settings -checkMember({ - conn: master, - name: "master2", +runHelloCmdAndAliases({ + conn: primary, + name: "primary2", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: true, secondary: false, @@ -228,15 +285,23 @@ checkMember({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: - ["arbiterOnly", "passives", "passive", "slaveDelay", "hidden", "buildIndexes"] + unwantedFields: [ + "isWritablePrimary", + "arbiterOnly", + "passives", + "passive", + "slaveDelay", + "secondaryDelaySecs", + "hidden", + "buildIndexes" + ] }); -checkMember({ +runHelloCmdAndAliases({ conn: replTest.liveNodes.slaves[0], - name: "first_slave", + name: "first_secondary", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -246,14 +311,21 @@ checkMember({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["arbiterOnly", "passives", "slaveDelayed", "buildIndexes"] + unwantedFields: [ + "isWritablePrimary", + "arbiterOnly", + "passives", + "slaveDelay", + "secondaryDelaySecs", + "buildIndexes" + ] }); -checkMember({ +runHelloCmdAndAliases({ conn: replTest.liveNodes.slaves[1], - name: "very_delayed_slave", + name: "very_delayed_secondary", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -265,14 +337,14 @@ checkMember({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["arbiterOnly", "passives"] + unwantedFields: ["isWritablePrimary", "arbiterOnly", "passives", "secondaryDelaySecs"] }); -checkMember({ +runHelloCmdAndAliases({ conn: replTest.liveNodes.slaves[2], name: "arbiter", goodValues: { - setName: "ismaster", + setName: "hello_and_aliases", setVersion: expectedVersion, ismaster: false, secondary: false, @@ -280,12 +352,20 @@ checkMember({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["slaveDelay", "hidden", "tags", "buildIndexes", "passive"] + unwantedFields: [ + "isWritablePrimary", + "slaveDelay", + "secondaryDelaySecs", + "hidden", + "tags", + "buildIndexes", + "passive" + ] }); // force reconfig and ensure all have the same setVersion afterwards -config = master.getDB("local").system.replset.findOne(); -master.getDB("admin").runCommand({replSetReconfig: config, force: true}); +config = primary.getDB("local").system.replset.findOne(); +primary.getDB("admin").runCommand({replSetReconfig: config, force: true}); assert.soon(function() { return agreeOnPrimaryAndSetVersion(); diff --git a/src/mongo/db/repl/is_master_response.cpp b/src/mongo/db/repl/is_master_response.cpp index 5b2c32486c8..34b39d33b8e 100644 --- a/src/mongo/db/repl/is_master_response.cpp +++ b/src/mongo/db/repl/is_master_response.cpp @@ -65,6 +65,8 @@ const std::string kLastWriteDateFieldName = "lastWriteDate"; const std::string kLastMajorityWriteOpTimeFieldName = "majorityOpTime"; const std::string kLastMajorityWriteDateFieldName = "majorityWriteDate"; const std::string kLastWriteFieldName = "lastWrite"; +const std::string kIsWritablePrimaryFieldName = "isWritablePrimary"; +const std::string kSecondaryDelaySecsFieldName = "secondaryDelaySecs"; // field name constants that don't directly correspond to member variables const std::string kInfoFieldName = "info"; @@ -102,7 +104,7 @@ IsMasterResponse::IsMasterResponse() _configSet(true), _shutdownInProgress(false) {} -void IsMasterResponse::addToBSON(BSONObjBuilder* builder) const { +void IsMasterResponse::addToBSON(BSONObjBuilder* builder, bool useLegacyResponseFields) const { if (_hostsSet) { std::vector<std::string> hosts; for (size_t i = 0; i < _hosts.size(); ++i) { @@ -136,7 +138,11 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder) const { } if (!_configSet) { - builder->append(kIsMasterFieldName, false); + if (useLegacyResponseFields) { + builder->append(kIsMasterFieldName, false); + } else { + builder->append(kIsWritablePrimaryFieldName, false); + } builder->append(kSecondaryFieldName, false); builder->append(kInfoFieldName, "Does not have a valid replica set config"); builder->append(kIsReplicaSetFieldName, true); @@ -146,7 +152,11 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder) const { invariant(_setVersionSet); builder->append(kSetVersionFieldName, static_cast<int>(_setVersion)); invariant(_isMasterSet); - builder->append(kIsMasterFieldName, _isMaster); + if (useLegacyResponseFields) { + builder->append(kIsMasterFieldName, _isMaster); + } else { + builder->append(kIsWritablePrimaryFieldName, _isMaster); + } invariant(_isSecondarySet); builder->append(kSecondaryFieldName, _secondary); @@ -160,8 +170,14 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder) const { builder->append(kHiddenFieldName, _hidden); if (_buildIndexesSet) builder->append(kBuildIndexesFieldName, _buildIndexes); - if (_slaveDelaySet) - builder->appendIntOrLL(kSlaveDelayFieldName, durationCount<Seconds>(_slaveDelay)); + if (_slaveDelaySet) { + if (useLegacyResponseFields) { + builder->appendIntOrLL(kSlaveDelayFieldName, durationCount<Seconds>(_slaveDelay)); + } else { + builder->appendIntOrLL(kSecondaryDelaySecsFieldName, + durationCount<Seconds>(_slaveDelay)); + } + } if (_tagsSet) { BSONObjBuilder tags(builder->subobjStart(kTagsFieldName)); for (unordered_map<std::string, std::string>::const_iterator it = _tags.begin(); @@ -188,9 +204,9 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder) const { } } -BSONObj IsMasterResponse::toBSON() const { +BSONObj IsMasterResponse::toBSON(bool useLegacyResponseFields) const { BSONObjBuilder builder; - addToBSON(&builder); + addToBSON(&builder, useLegacyResponseFields); return builder.obj(); } diff --git a/src/mongo/db/repl/is_master_response.h b/src/mongo/db/repl/is_master_response.h index ba3a033c371..dce40b840bd 100644 --- a/src/mongo/db/repl/is_master_response.h +++ b/src/mongo/db/repl/is_master_response.h @@ -62,19 +62,21 @@ public: Status initialize(const BSONObj& doc); /** - * Appends all non-default values to "builder". - * There are two values that are handled specially: if _inShutdown is true or _configSet - * is false, we will add a standard response to "builder" indicating either that we are - * in the middle of shutting down or we do not have a valid replica set config, ignoring - * the values of all other member variables. + * Appends all non-default values to "builder". When true, "useLegacyResponseFields" indicates + * that we are responding to an isMaster command and not a hello command. Attach the legacy + * "ismaster" field if true, and the "isWritablePrimary" field otherwise. There are two values + * that are handled specially: if _inShutdown is true or _configSet is false, we will add a + * standard response to "builder" indicating either that we are in the middle of shutting down + * or we do not have a valid replica set config, ignoring the values of all other member + * variables. */ - void addToBSON(BSONObjBuilder* builder) const; + void addToBSON(BSONObjBuilder* builder, bool useLegacyResponseFields) const; /** * Returns a BSONObj consisting the results of calling addToBSON on an otherwise empty * BSONObjBuilder. */ - BSONObj toBSON() const; + BSONObj toBSON(bool useLegacyResponseFields = true) const; // ===================== Accessors for member variables ================================= // diff --git a/src/mongo/db/repl/replication_info.cpp b/src/mongo/db/repl/replication_info.cpp index afb68ed3314..c3b740de811 100644 --- a/src/mongo/db/repl/replication_info.cpp +++ b/src/mongo/db/repl/replication_info.cpp @@ -76,13 +76,16 @@ constexpr auto kHelloString = "hello"_sd; constexpr auto kCamelCaseIsMasterString = "isMaster"_sd; constexpr auto kLowerCaseIsMasterString = "ismaster"_sd; -void appendReplicationInfo(OperationContext* opCtx, BSONObjBuilder& result, int level) { +void appendReplicationInfo(OperationContext* opCtx, + BSONObjBuilder& result, + int level, + bool useLegacyResponseFields) { ReplicationCoordinator* replCoord = getGlobalReplicationCoordinator(); if (replCoord->getSettings().usingReplSets()) { const auto& horizonParams = SplitHorizon::getParameters(opCtx->getClient()); IsMasterResponse isMasterResponse; replCoord->fillIsMasterForReplSet(&isMasterResponse, horizonParams); - result.appendElements(isMasterResponse.toBSON()); + result.appendElements(isMasterResponse.toBSON(useLegacyResponseFields)); if (level) { replCoord->appendSlaveInfoData(&result); } @@ -95,7 +98,7 @@ void appendReplicationInfo(OperationContext* opCtx, BSONObjBuilder& result, int string s = string("dead: ") + replAllDead; result.append("info", s); } else { - result.appendBool("ismaster", + result.appendBool((useLegacyResponseFields ? "ismaster" : "isWritablePrimary"), getGlobalReplicationCoordinator()->isMasterForReportingPurposes()); } @@ -178,7 +181,9 @@ public: int level = configElement.numberInt(); BSONObjBuilder result; - appendReplicationInfo(opCtx, result, level); + // TODO SERVER-50219: Change useLegacyResponseFields to false once the serverStatus changes + // to remove master-slave terminology are merged. + appendReplicationInfo(opCtx, result, level, true /* useLegacyResponseFields */); auto rbid = ReplicationProcess::get(opCtx)->getRollbackID(); if (ReplicationProcess::kUninitializedRollbackId != rbid) { @@ -252,6 +257,11 @@ public: LastError::get(opCtx->getClient()).disable(); } + // Parse the command name, which should be one of the following: hello, isMaster, or + // ismaster. If the command is "hello", we must attach an "isWritablePrimary" response field + // instead of "ismaster" and "secondaryDelaySecs" response field instead of "slaveDelay". + bool useLegacyResponseFields = (cmdObj.firstElementFieldName() != kHelloString); + transport::Session::TagMask sessionTagsToSet = 0; transport::Session::TagMask sessionTagsToUnset = 0; @@ -367,7 +377,7 @@ public: }); } - appendReplicationInfo(opCtx, result, 0); + appendReplicationInfo(opCtx, result, 0, useLegacyResponseFields); if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) { const int configServerModeNumber = 2; |