SERVER-49987 Rename response fields if hello command is sent on mongod

(cherry picked from commit e9d2fbd27574714fc3c2db7cd45d413b7fbd4718)

6 files changed, 198 insertions, 84 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 31cad5c2ff7..31b74a6d243 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -75,6 +75,8 @@ all:
     test_file: jstests/sharding/log_remote_op_wait.js
   - ticket: SERVER-49986
     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 5f309b83392..e9d5e344303 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));
     assert(res.connectionId, "connectionId missing or false" + tojson(res));
 
@@ -32,9 +37,10 @@ function checkResponseFields(commandString) {
             "passives",
             "arbiters",
             "primary",
-            "aribterOnly",
+            "arbiterOnly",
             "passive",
             "slaveDelay",
+            "secondaryDelaySecs",
             "hidden",
             "tags",
             "buildIndexes",
@@ -56,8 +62,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 3cd6cb0a8dc..f87f24cd41a 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();
 
@@ -95,54 +122,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._slaves[0],
-    name: "slave",
+    name: "secondary",
     goodValues: {
-        setName: "ismaster",
+        setName: "hello_and_aliases",
         setVersion: expectedVersion,
         ismaster: false,
         secondary: true,
@@ -151,14 +192,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._slaves[1],
-    name: "delayed_slave",
+    name: "delayed_secondary",
     goodValues: {
-        setName: "ismaster",
+        setName: "hello_and_aliases",
         setVersion: expectedVersion,
         ismaster: false,
         secondary: true,
@@ -169,14 +218,14 @@ checkMember({
     },
     wantedFields:
         ["hosts", "passives", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"],
-    unwantedFields: ["arbiterOnly", "tags"]
+    unwantedFields: ["isWritablePrimary", "arbiterOnly", "tags", "secondaryDelaySecs"]
 });
 
-checkMember({
+runHelloCmdAndAliases({
     conn: replTest._slaves[2],
     name: "arbiter",
     goodValues: {
-        setName: "ismaster",
+        setName: "hello_and_aliases",
         setVersion: expectedVersion,
         ismaster: false,
         secondary: false,
@@ -185,11 +234,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"
@@ -203,23 +260,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,
@@ -227,14 +284,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._slaves[0],
-    name: "first_slave",
+    name: "first_secondary",
     goodValues: {
-        setName: "ismaster",
+        setName: "hello_and_aliases",
         setVersion: expectedVersion,
         ismaster: false,
         secondary: true,
@@ -244,14 +310,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._slaves[1],
-    name: "very_delayed_slave",
+    name: "very_delayed_secondary",
     goodValues: {
-        setName: "ismaster",
+        setName: "hello_and_aliases",
         setVersion: expectedVersion,
         ismaster: false,
         secondary: true,
@@ -263,14 +336,14 @@ checkMember({
         ok: 1
     },
     wantedFields: ["hosts", "arbiters", "primary", "me", "maxBsonObjectSize", "localTime"],
-    unwantedFields: ["arbiterOnly", "passives"]
+    unwantedFields: ["isWritablePrimary", "arbiterOnly", "passives", "secondaryDelaySecs"]
 });
 
-checkMember({
+runHelloCmdAndAliases({
     conn: replTest._slaves[2],
     name: "arbiter",
     goodValues: {
-        setName: "ismaster",
+        setName: "hello_and_aliases",
         setVersion: expectedVersion,
         ismaster: false,
         secondary: false,
@@ -278,12 +351,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 5d377768cc6..96f7f465d9a 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 (_topologyVersion) {
         BSONObjBuilder topologyVersionBuilder(builder->subobjStart(kTopologyVersionFieldName));
         _topologyVersion->serialize(&topologyVersionBuilder);
@@ -141,7 +143,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);
@@ -151,7 +157,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);
 
@@ -165,8 +175,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 (stdx::unordered_map<std::string, std::string>::const_iterator it = _tags.begin();
@@ -193,9 +209,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 5ad203d45ee..9323c528db1 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 8685a30ecb8..b5b57cd9432 100644
--- a/src/mongo/db/repl/replication_info.cpp
+++ b/src/mongo/db/repl/replication_info.cpp
@@ -95,6 +95,7 @@ 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;
@@ -109,7 +110,7 @@ TopologyVersion appendReplicationInfo(OperationContext* opCtx,
         }
         auto isMasterResponse =
             replCoord->awaitIsMasterResponse(opCtx, horizonParams, clientTopologyVersion, deadline);
-        result.appendElements(isMasterResponse->toBSON());
+        result.appendElements(isMasterResponse->toBSON(useLegacyResponseFields));
         if (level) {
             replCoord->appendSlaveInfoData(&result);
         }
@@ -143,7 +144,7 @@ TopologyVersion appendReplicationInfo(OperationContext* opCtx,
         opCtx->sleepFor(Milliseconds(*maxAwaitTimeMS));
     }
 
-    result.appendBool("ismaster",
+    result.appendBool((useLegacyResponseFields ? "ismaster" : "isWritablePrimary"),
                       ReplicationCoordinator::get(opCtx)->isMasterForReportingPurposes());
 
     if (level) {
@@ -239,9 +240,12 @@ 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 */);
 
@@ -341,6 +345,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.firstElementFieldNameStringData() != kHelloString);
+
         transport::Session::TagMask sessionTagsToSet = 0;
         transport::Session::TagMask sessionTagsToUnset = 0;
 
@@ -483,8 +492,8 @@ public:
         }
 
         auto result = replyBuilder->getBodyBuilder();
-        auto currentTopologyVersion =
-            appendReplicationInfo(opCtx, result, 0, clientTopologyVersion, maxAwaitTimeMS);
+        auto currentTopologyVersion = appendReplicationInfo(
+            opCtx, result, 0, useLegacyResponseFields, clientTopologyVersion, maxAwaitTimeMS);
 
         if (serverGlobalParams.clusterRole == ClusterRole::ConfigServer) {
             const int configServerModeNumber = 2;