SERVER-50356: Account for last-continuous when closing connections on FCV upgrade/downgrade

jstests/noPassthroughWithMongod/fcv_changes_close_connections.js

@@ -0,0 +1,105 @@
+// Test closing connections from internal clients on featureCompatibilityVersion changes.
+(function() {
+"use strict";
+
+load("jstests/libs/parallel_shell_helpers.js");
+load("jstests/libs/fail_point_util.js");
+
+const adminDB = db.getSiblingDB("admin");
+const testDB = db.getSiblingDB("test");
+const collName = "coll";
+
+checkFCV(adminDB, latestFCV);
+assert.commandWorked(testDB.runCommand({insert: collName, documents: [{x: 1}]}));
+
+function testFCVChange({fcvDoc, expectConnectionClosed = true} = {}) {
+    let findCmdFailPoint = configureFailPoint(
+        db, "waitInFindBeforeMakingBatch", {nss: "test.coll", shouldCheckForInterrupt: true});
+
+    // Mimic running a find command from an internal client with a lower binary version.
+    function findWithLowerBinaryInternalClient(expectConnectionClosed) {
+        const testDB = db.getSiblingDB("test");
+
+        // The value of the 'internalClient.maxWireVersion' does not matter for the purpose of this
+        // test as long as it is less than the latest server's maxWireVersion. The server will tag
+        // this connection as an internal client connection with a lower binary version on receiving
+        // a 'hello' with 'internalClient.maxWireVersion' less than the server's maxWireVersion.
+        // Connections with such a tag are closed by the server on certain FCV changes.
+        //
+        // The 'hello' command below will succeed because this is run through the shell and the
+        // shell is always compatible talking to the server. In reality though, a real internal
+        // client with a lower binary version is expected to hang up immediately after receiving the
+        // response to the 'hello' command from a latest server with an upgraded FCV.
+        assert.commandWorked(testDB.adminCommand({
+            hello: 1,
+            internalClient: {minWireVersion: NumberInt(9), maxWireVersion: NumberInt(9)}
+        }));
+
+        // Since mongod expects other cluster members to always include explicit read/write concern
+        // (on commands that accept read/write concern), this test must be careful to mimic this
+        // behavior.
+        if (expectConnectionClosed) {
+            const e = assert.throws(
+                () => testDB.runCommand({find: "coll", readConcern: {level: "local"}}));
+            assert.includes(e.toString(), "network error while attempting to run command");
+        } else {
+            assert.commandWorked(testDB.runCommand({find: "coll", readConcern: {level: "local"}}));
+        }
+    }
+
+    const joinFindThread = startParallelShell(
+        funWithArgs(findWithLowerBinaryInternalClient, expectConnectionClosed), db.getMongo().port);
+
+    jsTestLog("Waiting for waitInFindBeforeMakingBatch failpoint");
+    findCmdFailPoint.wait();
+
+    jsTestLog("Updating featureCompatibilityVersion document to: " + tojson(fcvDoc));
+    assert.commandWorked(
+        adminDB.system.version.update({_id: "featureCompatibilityVersion"}, fcvDoc));
+
+    jsTestLog("Turning off waitInFindBeforeMakingBatch failpoint");
+    findCmdFailPoint.off();
+
+    jsTestLog("Joining findThread");
+    joinFindThread();
+}
+
+function runTest(oldVersion) {
+    jsTestLog("Testing with oldVersion: " + oldVersion);
+
+    // Downgrading to oldVersion.
+    // Test that connections from internal clients with lower binary versions are closed on FCV
+    // downgrade. In reality, there shouldn't be any long-lived open connections from internal
+    // clients with lower binary versions before the downgrade, because they are expected to hang up
+    // immediately after receiving the response to the initial 'hello' command from an incompatible
+    // server. But we still want to test that the server also try to close those incompatible
+    // connections proactively on FCV downgrade.
+    testFCVChange({
+        fcvDoc: {version: oldVersion, targetVersion: oldVersion, previousVersion: latestFCV},
+    });
+
+    // Downgraded to oldVersion.
+    // Test that connections from internal clients with lower binary versions are not closed on FCV
+    // fully downgraded to oldVersion.
+    testFCVChange({fcvDoc: {version: oldVersion}, expectConnectionClosed: false});
+
+    // Upgrading from oldVersion to 'latest'.
+    // Test that connections from internal clients with lower binary versions are closed on FCV
+    // upgrade.
+    testFCVChange({fcvDoc: {version: oldVersion, targetVersion: latestFCV}});
+
+    // Upgraded to 'latest'.
+    // Test that connections from internal clients with lower binary versions are closed on FCV
+    // fully upgraded to 'latest'.
+    testFCVChange({fcvDoc: {version: latestFCV}});
+}
+
+// Test upgrade/downgrade between 'latest' and 'last-continuous' if 'last-continuous' is not
+// 'last-lts'.
+if (lastContinuousFCV !== lastLTSFCV) {
+    runTest(lastContinuousFCV);
+}
+
+// Test upgrade/downgrade between 'latest' and 'last-lts'.
+runTest(lastLTSFCV);
+})();

jstests/noPassthroughWithMongod/isMaster_feature_compatibility_version.js

@@ -8,57 +8,79 @@
 
 const adminDB = db.getSiblingDB("admin");
 
-// This test manually runs isMaster with internalClient, which means that to the mongod, the
-// connection appears to be from another server. Since mongod expects other cluster members to
-// always include explicit read/write concern (on commands that accept read/write concern), this
-// test must be careful to mimic this behavior.
-const isMasterCommand = {
-    isMaster: 1,
-    internalClient: {minWireVersion: NumberInt(0), maxWireVersion: NumberInt(7)}
-};
+// This test manually runs isMaster with the 'internalClient' field, which means that to the
+// mongod, the connection appears to be from another server. This makes mongod to return an
+// isMaster response with a real 'minWireVersion' for internal clients instead of 0.
+//
+// The value of 'internalClient.maxWireVersion' in the isMaster command does not matter for the
+// purpose of this test and the isMaster command will succeed regardless because this is run
+// through the shell and the shell is always compatible talking to the server. In reality
+// though, a real internal client with a lower binary version is expected to hang up immediately
+// after receiving the response to the isMaster command from a latest server with an upgraded FCV.
+//
+// And we need to use a side connection to do so in order to prevent the test connection from
+// being closed on FCV changes.
+function isMasterAsInternalClient() {
+    const isMasterCommand = {
+        isMaster: 1,
+        internalClient: {minWireVersion: NumberInt(0), maxWireVersion: NumberInt(7)}
+    };
+    const connInternal = new Mongo(adminDB.getMongo().host);
+    const res = assert.commandWorked(connInternal.adminCommand(isMasterCommand));
+    connInternal.close();
+    return res;
+}
 
 // When the featureCompatibilityVersion is equal to the upgrade version, running isMaster with
-// internalClient returns minWireVersion == maxWireVersion.
+// internalClient returns a response with minWireVersion == maxWireVersion.
 checkFCV(adminDB, latestFCV);
-let res = adminDB.runCommand(isMasterCommand);
-assert.commandWorked(res);
+let res = isMasterAsInternalClient();
 assert.eq(res.minWireVersion, res.maxWireVersion, tojson(res));
 
-// When the featureCompatibilityVersion is upgrading, running isMaster with internalClient
-// returns minWireVersion == maxWireVersion.
-assert.commandWorked(
-    adminDB.system.version.update({_id: "featureCompatibilityVersion"},
-                                  {$set: {version: lastLTSFCV, targetVersion: latestFCV}},
-                                  {writeConcern: {w: 1}}));
-res = adminDB.runCommand(isMasterCommand);
-assert.commandWorked(res);
-assert.eq(res.minWireVersion, res.maxWireVersion, tojson(res));
+// Test wire version for upgrade/downgrade.
+function runTest(downgradeFCV, downgradeWireVersion, maxWireVersion) {
+    // When the featureCompatibilityVersion is upgrading, running isMaster with internalClient
+    // returns a response with minWireVersion == maxWireVersion.
+    assert.commandWorked(
+        adminDB.system.version.update({_id: "featureCompatibilityVersion"},
+                                      {$set: {version: downgradeFCV, targetVersion: latestFCV}}));
+    let res = isMasterAsInternalClient();
+    assert.eq(res.minWireVersion, res.maxWireVersion, tojson(res));
+    assert.eq(maxWireVersion, res.maxWireVersion, tojson(res));
 
-// When the featureCompatibilityVersion is downgrading, running isMaster with internalClient
-// returns minWireVersion == maxWireVersion.
-assert.commandWorked(adminDB.system.version.update(
-    {_id: "featureCompatibilityVersion"},
-    {$set: {version: lastLTSFCV, targetVersion: lastLTSFCV, previousVersion: latestFCV}},
-    {writeConcern: {w: 1}}));
-res = adminDB.runCommand(isMasterCommand);
-assert.commandWorked(res);
-assert.eq(res.minWireVersion, res.maxWireVersion, tojson(res));
+    // When the featureCompatibilityVersion is downgrading, running isMaster with internalClient
+    // returns a response with minWireVersion == maxWireVersion.
+    assert.commandWorked(adminDB.system.version.update(
+        {_id: "featureCompatibilityVersion"},
+        {$set: {version: downgradeFCV, targetVersion: downgradeFCV, previousVersion: latestFCV}}));
+    res = isMasterAsInternalClient();
+    assert.eq(res.minWireVersion, res.maxWireVersion, tojson(res));
+    assert.eq(maxWireVersion, res.maxWireVersion, tojson(res));
 
-// When the featureCompatibilityVersion is equal to the downgrade version, running isMaster with
-// internalClient returns minWireVersion + 1 == maxWireVersion.
-assert.commandWorked(
-    adminDB.runCommand({setFeatureCompatibilityVersion: lastLTSFCV, writeConcern: {w: 1}}));
-res = adminDB.runCommand(isMasterCommand);
-assert.commandWorked(res);
-assert.eq(res.minWireVersion + 1, res.maxWireVersion, tojson(res));
+    // When the featureCompatibilityVersion is equal to the downgrade version, running isMaster with
+    // internalClient returns a response with minWireVersion + 1 == maxWireVersion.
+    assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: downgradeFCV}));
+    res = isMasterAsInternalClient();
+    assert.eq(downgradeWireVersion, res.minWireVersion, tojson(res));
+    assert.eq(maxWireVersion, res.maxWireVersion, tojson(res));
 
-// When the internalClient field is missing from the isMaster command, the response returns the
-// full wire version range from minWireVersion == 0 to maxWireVersion == latest version, even if
-// the featureCompatibilityVersion is equal to the upgrade version.
-assert.commandWorked(
-    adminDB.runCommand({setFeatureCompatibilityVersion: latestFCV, writeConcern: {w: 1}}));
-res = adminDB.runCommand({isMaster: 1});
-assert.commandWorked(res);
-assert.eq(res.minWireVersion, 0, tojson(res));
-assert.lt(res.minWireVersion, res.maxWireVersion, tojson(res));
+    // When the internalClient field is missing from the isMaster command, the response returns the
+    // full wire version range from minWireVersion == 0 to maxWireVersion == latest version, even if
+    // the featureCompatibilityVersion is equal to the upgrade version.
+    assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: latestFCV}));
+    res = adminDB.runCommand({isMaster: 1});
+    assert.commandWorked(res);
+    assert.eq(res.minWireVersion, 0, tojson(res));
+    assert.lt(res.minWireVersion, res.maxWireVersion, tojson(res));
+    assert.eq(maxWireVersion, res.maxWireVersion, tojson(res));
+}
+
+// Test upgrade/downgrade between 'latest' and 'last-continuous' if 'last-continuous' is not
+// 'last-lts'.
+if (lastContinuousFCV !== lastLTSFCV) {
+    runTest(lastContinuousFCV, res.maxWireVersion - 1, res.maxWireVersion);
+}
+
+// Test upgrade/downgrade between 'latest' and 'last-lts'.
+runTest(lastLTSFCV, res.maxWireVersion - numVersionsSinceLastLTS, res.maxWireVersion);
 })();

jstests/replsets/awaitable_ismaster_fcv_change.js

@@ -56,16 +56,32 @@ function runAwaitableIsMasterBeforeFCVChange(
 
 function runTest(downgradeFCV) {
     jsTestLog("Running test with downgradeFCV: " + downgradeFCV);
-    // This test manually runs isMaster with internalClient, which means that to the mongod, the
-    // connection appears to be from another server. Since mongod expects other cluster members to
-    // always include explicit read/write concern (on commands that accept read/write concern), this
-    // test must be careful to mimic this behavior.
+
+    // This test manually runs isMaster with the 'internalClient' field, which means that to the
+    // mongod, the connection appears to be from another server. This makes mongod to return an
+    // isMaster response with a real 'minWireVersion' for internal clients instead of 0.
+    //
+    // The value of 'internalClient.maxWireVersion' in the isMaster command does not matter for the
+    // purpose of this test and the isMaster command will succeed regardless because this is run
+    // through the shell and the shell is always compatible talking to the server. In reality
+    // though, a real internal client with a lower binary version is expected to hang up immediately
+    // after receiving the response to the isMaster command from a latest server with an upgraded
+    // FCV.
+    //
+    // And we need to use a side connection to do so in order to prevent the test connection from
+    // being closed on FCV changes.
+    function isMasterAsInternalClient() {
+        let connInternal = new Mongo(primary.host);
+        const res = assert.commandWorked(connInternal.adminCommand({
+            isMaster: 1,
+            internalClient: {minWireVersion: NumberInt(0), maxWireVersion: NumberInt(9)}
+        }));
+        connInternal.close();
+        return res;
+    }
 
     // Get the server topologyVersion, minWireVersion, and maxWireversion.
-    const primaryResult = assert.commandWorked(primaryAdminDB.runCommand({
-        isMaster: 1,
-        internalClient: {minWireVersion: NumberInt(0), maxWireVersion: NumberInt(9)}
-    }));
+    const primaryResult = isMasterAsInternalClient();
     assert(primaryResult.hasOwnProperty("topologyVersion"), tojson(primaryResult));
 
     const maxWireVersion = primaryResult.maxWireVersion;
@@ -117,8 +133,7 @@ function runTest(downgradeFCV) {
     assert.eq(1, numAwaitingTopologyChangeOnSecondary);
 
     // Setting the FCV to the same version will not trigger an isMaster response.
-    assert.commandWorked(primaryAdminDB.runCommand(
-        {setFeatureCompatibilityVersion: latestFCV, writeConcern: {w: 1}}));
+    assert.commandWorked(primaryAdminDB.runCommand({setFeatureCompatibilityVersion: latestFCV}));
     checkFCV(primaryAdminDB, latestFCV);
     checkFCV(secondaryAdminDB, latestFCV);
 
@@ -133,8 +148,7 @@ function runTest(downgradeFCV) {
     jsTestLog("Downgrade the featureCompatibilityVersion.");
     // Downgrading the FCV will cause the isMaster requests to respond on both primary and
     // secondary.
-    assert.commandWorked(primaryAdminDB.runCommand(
-        {setFeatureCompatibilityVersion: downgradeFCV, writeConcern: {w: 1}}));
+    assert.commandWorked(primaryAdminDB.runCommand({setFeatureCompatibilityVersion: downgradeFCV}));
     awaitIsMasterBeforeDowngradeFCVOnPrimary();
     awaitIsMasterBeforeDowngradeFCVOnSecondary();
     // Ensure the featureCompatibilityVersion document update has been replicated.
@@ -151,10 +165,7 @@ function runTest(downgradeFCV) {
     assert.eq(0, numAwaitingTopologyChangeOnSecondary);
 
     // Get the new topologyVersion.
-    const primaryResponseAfterDowngrade = assert.commandWorked(primaryAdminDB.runCommand({
-        isMaster: 1,
-        internalClient: {minWireVersion: NumberInt(0), maxWireVersion: NumberInt(maxWireVersion)}
-    }));
+    const primaryResponseAfterDowngrade = isMasterAsInternalClient();
     assert(primaryResponseAfterDowngrade.hasOwnProperty("topologyVersion"),
            tojson(primaryResponseAfterDowngrade));
     const primaryTopologyVersionAfterDowngrade = primaryResponseAfterDowngrade.topologyVersion;
@@ -197,8 +208,7 @@ function runTest(downgradeFCV) {
     assert.eq(1, numAwaitingTopologyChangeOnSecondary);
 
     // Setting the FCV to the same version will not trigger an isMaster response.
-    assert.commandWorked(primaryAdminDB.runCommand(
-        {setFeatureCompatibilityVersion: downgradeFCV, writeConcern: {w: 1}}));
+    assert.commandWorked(primaryAdminDB.runCommand({setFeatureCompatibilityVersion: downgradeFCV}));
     checkFCV(primaryAdminDB, downgradeFCV);
     checkFCV(secondaryAdminDB, downgradeFCV);
 
@@ -212,8 +222,7 @@ function runTest(downgradeFCV) {
 
     jsTestLog("Upgrade the featureCompatibilityVersion.");
     // Upgrading the FCV will cause the isMaster requests to respond on both primary and secondary.
-    assert.commandWorked(primaryAdminDB.runCommand(
-        {setFeatureCompatibilityVersion: latestFCV, writeConcern: {w: 1}}));
+    assert.commandWorked(primaryAdminDB.runCommand({setFeatureCompatibilityVersion: latestFCV}));
     awaitIsMasterBeforeUpgradeFCVOnPrimary();
     awaitIsMasterBeforeUpgradeFCVOnSecondary();
     // Ensure the featureCompatibilityVersion document update has been replicated.

src/mongo/db/commands/feature_compatibility_version.cpp

@@ -214,15 +214,16 @@ bool FeatureCompatibilityVersion::isCleanStartUp() {
 void FeatureCompatibilityVersion::updateMinWireVersion() {
     WireSpec& wireSpec = WireSpec::instance();
 
-    if (serverGlobalParams.featureCompatibility.isGreaterThan(
-            FeatureCompatibilityParams::kLastContinuous)) {
-        // FCV == kLatest
+    if (serverGlobalParams.featureCompatibility.isGreaterThanOrEqualTo(
+            FeatureCompatibilityParams::kLatest) ||
+        serverGlobalParams.featureCompatibility.isUpgradingOrDowngrading()) {
+        // FCV == kLatest or FCV is upgrading/downgrading.
         WireSpec::Specification newSpec = *wireSpec.get();
         newSpec.incomingInternalClient.minWireVersion = LATEST_WIRE_VERSION;
         newSpec.outgoing.minWireVersion = LATEST_WIRE_VERSION;
         wireSpec.reset(std::move(newSpec));
-    } else if (serverGlobalParams.featureCompatibility.isGreaterThan(
-                   FeatureCompatibilityParams::kLastLTS)) {
+    } else if (serverGlobalParams.featureCompatibility.isGreaterThanOrEqualTo(
+                   FeatureCompatibilityParams::kLastContinuous)) {
         // FCV == kLastContinuous
         WireSpec::Specification newSpec = *wireSpec.get();
         newSpec.incomingInternalClient.minWireVersion = LAST_CONT_WIRE_VERSION;

src/mongo/db/fcv_op_observer.cpp

@@ -56,7 +56,10 @@ void FcvOpObserver::_setVersion(OperationContext* opCtx,
     FeatureCompatibilityVersion::updateMinWireVersion();
 
     // (Generic FCV reference): This FCV check should exist across LTS binary versions.
-    if (newVersion != ServerGlobalParams::FeatureCompatibility::kLastLTS) {
+    if (serverGlobalParams.featureCompatibility.isGreaterThanOrEqualTo(
+            FeatureCompatibilityParams::kLatest) ||
+        serverGlobalParams.featureCompatibility.isUpgradingOrDowngrading()) {
+        // minWireVersion == maxWireVersion on kLatest FCV or upgrading/downgrading FCV.
         // Close all incoming connections from internal clients with binary versions lower than
         // ours.
         opCtx->getServiceContext()->getServiceEntryPoint()->endAllSessions(

src/mongo/db/repl/replication_info.cpp

@@ -325,6 +325,7 @@ public:
         auto internalClientElement = cmdObj["internalClient"];
         if (internalClientElement) {
             sessionTagsToSet |= transport::Session::kInternalClient;
+            sessionTagsToUnset |= transport::Session::kExternalClientKeepOpen;
 
             uassert(ErrorCodes::TypeMismatch,
                     str::stream() << "'internalClient' must be of type Object, but was of type "