diff options
author | Pavi Vetriselvan <pavithra.vetriselvan@mongodb.com> | 2020-08-26 18:51:54 -0400 |
---|---|---|
committer | Pavi Vetriselvan <pavithra.vetriselvan@mongodb.com> | 2020-08-26 18:51:54 -0400 |
commit | 2281bdba9429a39a4cef57d2f287459976c10382 (patch) | |
tree | 505bc5bc89c96cfa12591a556f837bada9282219 | |
parent | dba4734316f55fddb4fb3fae6cd541a18ad676bc (diff) | |
download | mongo-2281bdba9429a39a4cef57d2f287459976c10382.tar.gz |
Revert "SERVER-49987 Rename response fields if hello command is sent on mongod"
This reverts commit 7157e90233ddc3b1b208564a711289a2d99e5ee5.
-rw-r--r-- | etc/backports_required_for_multiversion_tests.yml | 2 | ||||
-rw-r--r-- | jstests/core/ismaster.js | 20 | ||||
-rw-r--r-- | jstests/replsets/ismaster1.js | 197 | ||||
-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 | 17 |
6 files changed, 84 insertions, 198 deletions
diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml index b16fe6aabf9..a315ca74c90 100644 --- a/etc/backports_required_for_multiversion_tests.yml +++ b/etc/backports_required_for_multiversion_tests.yml @@ -75,8 +75,6 @@ all: test_file: jstests/sharding/log_remote_op_wait.js - ticket: SERVER-49987 test_file: jstests/core/ismaster.js - - ticket: SERVER-49987 - test_file: jstests/replsets/ismaster1.js - ticket: SERVER-49986 test_file: jstests/core/views/views_all_commands.js - ticket: SERVER-49986 diff --git a/jstests/core/ismaster.js b/jstests/core/ismaster.js index e9d5e344303..5f309b83392 100644 --- a/jstests/core/ismaster.js +++ b/jstests/core/ismaster.js @@ -14,16 +14,11 @@ 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)); - 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)); - } + // 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)); assert(res.localTime, "localTime possibly missing:" + tojson(res)); assert(res.connectionId, "connectionId missing or false" + tojson(res)); @@ -37,10 +32,9 @@ function checkResponseFields(commandString) { "passives", "arbiters", "primary", - "arbiterOnly", + "aribterOnly", "passive", "slaveDelay", - "secondaryDelaySecs", "hidden", "tags", "buildIndexes", @@ -62,6 +56,8 @@ function checkResponseFields(commandString) { } } -checkResponseFields("hello"); +// TODO SERVER-49987: add the "hello" command test back in once mongod response fields are +// appropriately changed. +// checkResponseFields("hello"); checkResponseFields("ismaster"); checkResponseFields("isMaster"); diff --git a/jstests/replsets/ismaster1.js b/jstests/replsets/ismaster1.js index f87f24cd41a..3cd6cb0a8dc 100644 --- a/jstests/replsets/ismaster1.js +++ b/jstests/replsets/ismaster1.js @@ -1,7 +1,6 @@ /** - * 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. + * 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 */ // Skip db hash check because node 2 is slave delayed and may time out on awaitReplication. @@ -29,36 +28,10 @@ var generateErrorString = function(badFields, missingFields, badValues, result) return str; }; -// 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"); - } - } +// function to check a single result +var checkMember = function(memberInfo) { + // run isMaster on the connection + result = memberInfo.conn.getDB("admin").runCommand({isMaster: 1}); // make sure result doesn't contain anything it shouldn't var badFields = []; @@ -105,7 +78,7 @@ var checkResponseFields = function(memberInfo, cmd) { }; // start of test code -var name = "hello_and_aliases"; +var name = "ismaster"; var replTest = new ReplSetTest({name: name, nodes: 4}); var nodes = replTest.startSet(); @@ -122,68 +95,54 @@ var agreeOnPrimaryAndSetVersion = function(setVersion) { print("Waiting for primary and replica set version " + setVersion); var nodes = replTest.nodes; - var currPrimary = undefined; + var primary = undefined; var lastSetVersion = setVersion; for (var i = 0; i < nodes.length; i++) { try { - var helloResult = nodes[i].getDB("admin").runCommand({hello: 1}); + var isMasterResult = nodes[i].getDB("admin").runCommand({isMaster: 1}); } catch (e) { // handle reconnect errors due to step downs - print("Error while calling hello on " + nodes[i] + ": " + e); + print("Error while calling isMaster on " + nodes[i] + ": " + e); return false; } - printjson(helloResult); - if (!currPrimary) - currPrimary = helloResult.primary; + printjson(isMasterResult); + if (!primary) + primary = isMasterResult.primary; if (!lastSetVersion) - lastSetVersion = helloResult.setVersion; - if (helloResult.primary != currPrimary || !currPrimary) + lastSetVersion = isMasterResult.setVersion; + if (isMasterResult.primary != primary || !primary) return false; - if (helloResult.setVersion != lastSetVersion) + if (isMasterResult.setVersion != lastSetVersion) return false; } return true; }; -var primary = replTest.getPrimary(); +var master = 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 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 - }, +// 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}, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: [ - "isWritablePrimary", - "arbiterOnly", - "passive", - "slaveDelay", - "secondaryDelaySecs", - "hidden", - "tags", - "buildIndexes" - ] + unwantedFields: ["arbiterOnly", "passive", "slaveDelay", "hidden", "tags", "buildIndexes"] }); -runHelloCmdAndAliases({ +checkMember({ conn: replTest._slaves[0], - name: "secondary", + name: "slave", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -192,22 +151,14 @@ runHelloCmdAndAliases({ }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: [ - "isWritablePrimary", - "arbiterOnly", - "slaveDelay", - "secondaryDelaySecs", - "hidden", - "tags", - "buildIndexes" - ] + unwantedFields: ["arbiterOnly", "slaveDelay", "hidden", "tags", "buildIndexes"] }); -runHelloCmdAndAliases({ +checkMember({ conn: replTest._slaves[1], - name: "delayed_secondary", + name: "delayed_slave", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -218,14 +169,14 @@ runHelloCmdAndAliases({ }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["isWritablePrimary", "arbiterOnly", "tags", "secondaryDelaySecs"] + unwantedFields: ["arbiterOnly", "tags"] }); -runHelloCmdAndAliases({ +checkMember({ conn: replTest._slaves[2], name: "arbiter", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: false, secondary: false, @@ -234,19 +185,11 @@ runHelloCmdAndAliases({ }, wantedFields: ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: [ - "isWritablePrimary", - "slaveDelay", - "secondaryDelaySecs", - "hidden", - "tags", - "buildIndexes", - "passive" - ] + unwantedFields: ["slaveDelay", "hidden", "tags", "buildIndexes", "passive"] }); -// Reconfigure the replset and make sure the changes are present on all members. -config = primary.getDB("local").system.replset.findOne(); +// reconfigure and make sure the changes show up in ismaster on all members +config = master.getDB("local").system.replset.findOne(); config.version = config.version + 1; config.members[0].tags = { disk: "ssd" @@ -260,23 +203,23 @@ config.members[2].tags = { disk: "hdd" }; try { - result = primary.getDB("admin").runCommand({replSetReconfig: config}); + result = master.getDB("admin").runCommand({replSetReconfig: config}); } catch (e) { print(e); } -primary = replTest.getPrimary(); +master = 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 -runHelloCmdAndAliases({ - conn: primary, - name: "primary2", +checkMember({ + conn: master, + name: "master2", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: true, secondary: false, @@ -284,23 +227,14 @@ runHelloCmdAndAliases({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: [ - "isWritablePrimary", - "arbiterOnly", - "passives", - "passive", - "slaveDelay", - "secondaryDelaySecs", - "hidden", - "buildIndexes" - ] + unwantedFields: ["arbiterOnly", "passives", "passive", "slaveDelay", "hidden", "buildIndexes"] }); -runHelloCmdAndAliases({ +checkMember({ conn: replTest._slaves[0], - name: "first_secondary", + name: "first_slave", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -310,21 +244,14 @@ runHelloCmdAndAliases({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: [ - "isWritablePrimary", - "arbiterOnly", - "passives", - "slaveDelay", - "secondaryDelaySecs", - "buildIndexes" - ] + unwantedFields: ["arbiterOnly", "passives", "slaveDelayed", "buildIndexes"] }); -runHelloCmdAndAliases({ +checkMember({ conn: replTest._slaves[1], - name: "very_delayed_secondary", + name: "very_delayed_slave", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: false, secondary: true, @@ -336,14 +263,14 @@ runHelloCmdAndAliases({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: ["isWritablePrimary", "arbiterOnly", "passives", "secondaryDelaySecs"] + unwantedFields: ["arbiterOnly", "passives"] }); -runHelloCmdAndAliases({ +checkMember({ conn: replTest._slaves[2], name: "arbiter", goodValues: { - setName: "hello_and_aliases", + setName: "ismaster", setVersion: expectedVersion, ismaster: false, secondary: false, @@ -351,20 +278,12 @@ runHelloCmdAndAliases({ ok: 1 }, wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"], - unwantedFields: [ - "isWritablePrimary", - "slaveDelay", - "secondaryDelaySecs", - "hidden", - "tags", - "buildIndexes", - "passive" - ] + unwantedFields: ["slaveDelay", "hidden", "tags", "buildIndexes", "passive"] }); // force reconfig and ensure all have the same setVersion afterwards -config = primary.getDB("local").system.replset.findOne(); -primary.getDB("admin").runCommand({replSetReconfig: config, force: true}); +config = master.getDB("local").system.replset.findOne(); +master.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 96f7f465d9a..5d377768cc6 100644 --- a/src/mongo/db/repl/is_master_response.cpp +++ b/src/mongo/db/repl/is_master_response.cpp @@ -65,8 +65,6 @@ 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"; @@ -104,7 +102,7 @@ IsMasterResponse::IsMasterResponse() _configSet(true), _shutdownInProgress(false) {} -void IsMasterResponse::addToBSON(BSONObjBuilder* builder, bool useLegacyResponseFields) const { +void IsMasterResponse::addToBSON(BSONObjBuilder* builder) const { if (_topologyVersion) { BSONObjBuilder topologyVersionBuilder(builder->subobjStart(kTopologyVersionFieldName)); _topologyVersion->serialize(&topologyVersionBuilder); @@ -143,11 +141,7 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder, bool useLegacyResponse } if (!_configSet) { - if (useLegacyResponseFields) { - builder->append(kIsMasterFieldName, false); - } else { - builder->append(kIsWritablePrimaryFieldName, false); - } + builder->append(kIsMasterFieldName, false); builder->append(kSecondaryFieldName, false); builder->append(kInfoFieldName, "Does not have a valid replica set config"); builder->append(kIsReplicaSetFieldName, true); @@ -157,11 +151,7 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder, bool useLegacyResponse invariant(_setVersionSet); builder->append(kSetVersionFieldName, static_cast<int>(_setVersion)); invariant(_isMasterSet); - if (useLegacyResponseFields) { - builder->append(kIsMasterFieldName, _isMaster); - } else { - builder->append(kIsWritablePrimaryFieldName, _isMaster); - } + builder->append(kIsMasterFieldName, _isMaster); invariant(_isSecondarySet); builder->append(kSecondaryFieldName, _secondary); @@ -175,14 +165,8 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder, bool useLegacyResponse builder->append(kHiddenFieldName, _hidden); if (_buildIndexesSet) builder->append(kBuildIndexesFieldName, _buildIndexes); - if (_slaveDelaySet) { - if (useLegacyResponseFields) { - builder->appendIntOrLL(kSlaveDelayFieldName, durationCount<Seconds>(_slaveDelay)); - } else { - builder->appendIntOrLL(kSecondaryDelaySecsFieldName, - durationCount<Seconds>(_slaveDelay)); - } - } + if (_slaveDelaySet) + builder->appendIntOrLL(kSlaveDelayFieldName, durationCount<Seconds>(_slaveDelay)); if (_tagsSet) { BSONObjBuilder tags(builder->subobjStart(kTagsFieldName)); for (stdx::unordered_map<std::string, std::string>::const_iterator it = _tags.begin(); @@ -209,9 +193,9 @@ void IsMasterResponse::addToBSON(BSONObjBuilder* builder, bool useLegacyResponse } } -BSONObj IsMasterResponse::toBSON(bool useLegacyResponseFields) const { +BSONObj IsMasterResponse::toBSON() const { BSONObjBuilder builder; - addToBSON(&builder, useLegacyResponseFields); + addToBSON(&builder); return builder.obj(); } diff --git a/src/mongo/db/repl/is_master_response.h b/src/mongo/db/repl/is_master_response.h index 9323c528db1..5ad203d45ee 100644 --- a/src/mongo/db/repl/is_master_response.h +++ b/src/mongo/db/repl/is_master_response.h @@ -62,21 +62,19 @@ public: Status initialize(const BSONObj& doc); /** - * 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. + * 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. */ - void addToBSON(BSONObjBuilder* builder, bool useLegacyResponseFields) const; + void addToBSON(BSONObjBuilder* builder) const; /** * Returns a BSONObj consisting the results of calling addToBSON on an otherwise empty * BSONObjBuilder. */ - BSONObj toBSON(bool useLegacyResponseFields = true) const; + BSONObj toBSON() const; // ===================== Accessors for member variables ================================= // diff --git a/src/mongo/db/repl/replication_info.cpp b/src/mongo/db/repl/replication_info.cpp index b5b57cd9432..8685a30ecb8 100644 --- a/src/mongo/db/repl/replication_info.cpp +++ b/src/mongo/db/repl/replication_info.cpp @@ -95,7 +95,6 @@ constexpr auto kLowerCaseIsMasterString = "ismaster"_sd; TopologyVersion appendReplicationInfo(OperationContext* opCtx, BSONObjBuilder& result, int level, - bool useLegacyResponseFields, boost::optional<TopologyVersion> clientTopologyVersion, boost::optional<long long> maxAwaitTimeMS) { TopologyVersion topologyVersion; @@ -110,7 +109,7 @@ TopologyVersion appendReplicationInfo(OperationContext* opCtx, } auto isMasterResponse = replCoord->awaitIsMasterResponse(opCtx, horizonParams, clientTopologyVersion, deadline); - result.appendElements(isMasterResponse->toBSON(useLegacyResponseFields)); + result.appendElements(isMasterResponse->toBSON()); if (level) { replCoord->appendSlaveInfoData(&result); } @@ -144,7 +143,7 @@ TopologyVersion appendReplicationInfo(OperationContext* opCtx, opCtx->sleepFor(Milliseconds(*maxAwaitTimeMS)); } - result.appendBool((useLegacyResponseFields ? "ismaster" : "isWritablePrimary"), + result.appendBool("ismaster", ReplicationCoordinator::get(opCtx)->isMasterForReportingPurposes()); if (level) { @@ -240,12 +239,9 @@ public: int level = configElement.numberInt(); BSONObjBuilder result; - // TODO SERVER-50219: Change useLegacyResponseFields to false once the serverStatus changes - // to remove master-slave terminology are merged. appendReplicationInfo(opCtx, result, level, - true /* useLegacyResponseFields */, boost::none /* clientTopologyVersion */, boost::none /* maxAwaitTimeMS */); @@ -345,11 +341,6 @@ 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.firstElementFieldNameStringData() != kHelloString); - transport::Session::TagMask sessionTagsToSet = 0; transport::Session::TagMask sessionTagsToUnset = 0; @@ -492,8 +483,8 @@ public: } auto result = replyBuilder->getBodyBuilder(); - auto currentTopologyVersion = appendReplicationInfo( - opCtx, result, 0, useLegacyResponseFields, clientTopologyVersion, maxAwaitTimeMS); + auto currentTopologyVersion = + appendReplicationInfo(opCtx, result, 0, clientTopologyVersion, maxAwaitTimeMS); if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) { const int configServerModeNumber = 2; |