SERVER-34459 Clear in-memory database versions on setFCV downgrade on shard primaries and secondaries

7 files changed, 204 insertions, 56 deletions

diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
index 4b291847bf0..0b2804c05cd 100644
--- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
+++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml
@@ -9,6 +9,8 @@ selector:
   # running in fully downgraded version, the config server won't forward the
   # command to the shards - it'll just return success immediately. 
   - jstests/sharding/max_time_ms_sharded_new_commands.js
+  # Calls setFCV=4.0, which cannot be run on last-stable shards.
+  - jstests/sharding/database_versioning_upgrade_downgrade.js
   #### Enable when 4.0 becomes last-stable.
   - jstests/sharding/change_streams_unsharded_becomes_sharded.js
   - jstests/sharding/create_database.js
diff --git a/jstests/libs/database_versioning.js b/jstests/libs/database_versioning.js
new file mode 100644
index 00000000000..b5d3f1d3e8a
--- /dev/null
+++ b/jstests/libs/database_versioning.js
@@ -0,0 +1,26 @@
+// Provides methods for inspecting the in-memory and on-disk database caches on shard nodes.
+
+function checkInMemoryDatabaseVersion(conn, dbName, expectedVersion) {
+    const res = conn.adminCommand({getDatabaseVersion: dbName});
+    assert.commandWorked(res);
+    assert.docEq(res.dbVersion,
+                 expectedVersion,
+                 conn + " did not have expected in-memory database version for " + dbName);
+}
+
+function checkOnDiskDatabaseVersion(conn, dbName, authoritativeEntry) {
+    const res = conn.getDB("config").runCommand({find: "cache.databases", filter: {_id: dbName}});
+    assert.commandWorked(res);
+    const cacheEntry = res.cursor.firstBatch[0];
+
+    if (authoritativeEntry === undefined) {
+        assert.eq(cacheEntry, undefined);
+    } else {
+        // Remove the 'enterCriticalSectionCounter' field, which is the only field the cache
+        // entry should have but the authoritative entry does not.
+        delete cacheEntry["enterCriticalSectionCounter"];
+        assert.docEq(cacheEntry,
+                     authoritativeEntry,
+                     conn + " did not have expected on-disk database version for " + dbName);
+    }
+}
diff --git a/jstests/sharding/database_versioning_safe_secondary_reads.js b/jstests/sharding/database_versioning_safe_secondary_reads.js
index 9123cc91c8a..71386b0f968 100644
--- a/jstests/sharding/database_versioning_safe_secondary_reads.js
+++ b/jstests/sharding/database_versioning_safe_secondary_reads.js
@@ -7,32 +7,7 @@
  */
 (function() {
     "use strict";
-
-    function checkInMemoryDatabaseVersion(conn, dbName, expectedVersion) {
-        const res = conn.adminCommand({getDatabaseVersion: dbName});
-        assert.commandWorked(res);
-        assert.docEq(res.dbVersion,
-                     expectedVersion,
-                     conn + " did not have expected in-memory database version for " + dbName);
-    }
-
-    function checkOnDiskDatabaseVersion(conn, dbName, authoritativeEntry) {
-        const res =
-            conn.getDB("config").runCommand({find: "cache.databases", filter: {_id: dbName}});
-        assert.commandWorked(res);
-        const cacheEntry = res.cursor.firstBatch[0];
-
-        if (authoritativeEntry === undefined) {
-            assert.eq(cacheEntry, undefined);
-        } else {
-            // Remove the 'enterCriticalSectionCounter' field, which is the only field the cache
-            // entry should have but the authoritative entry does not.
-            delete cacheEntry["enterCriticalSectionCounter"];
-            assert.docEq(cacheEntry,
-                         authoritativeEntry,
-                         conn + " did not have expected on-disk database version for " + dbName);
-        }
-    }
+    load("jstests/libs/database_versioning.js");
 
     const dbName = "test";
 
diff --git a/jstests/sharding/database_versioning_upgrade_downgrade.js b/jstests/sharding/database_versioning_upgrade_downgrade.js
new file mode 100644
index 00000000000..57764563638
--- /dev/null
+++ b/jstests/sharding/database_versioning_upgrade_downgrade.js
@@ -0,0 +1,117 @@
+/**
+ * Tests that
+ * 1) the 'config.databases' schema gets upgraded/downgraded on setFCV
+ * 2) shards' in-memory cached database versions get cleared on FCV downgrade (on both the primary
+      and secondary nodes)
+ * 3) shards only cache the database version in-memory in FCV 4.0.
+ */
+(function() {
+    "use strict";
+
+    load("jstests/libs/feature_compatibility_version.js");
+    load("jstests/libs/database_versioning.js");
+
+    const st = new ShardingTest(
+        {shards: {rs0: {nodes: [{rsConfig: {votes: 1}}, {rsConfig: {priority: 0, votes: 0}}]}}});
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: lastStableFCV}));
+
+    const db1Name = "db1";
+    const db2Name = "db2";
+    const collName = "foo";
+    const ns = db1Name + "." + collName;
+
+    // Create both databases in the sharding catalog.
+    assert.commandWorked(st.s.adminCommand({enableSharding: db1Name}));
+    assert.commandWorked(st.s.adminCommand({enableSharding: db2Name}));
+
+    // Ensure neither database entry has a database version.
+    const db1EntryOriginal =
+        st.s.getDB("config").getCollection("databases").findOne({_id: db1Name});
+    const db2EntryOriginal =
+        st.s.getDB("config").getCollection("databases").findOne({_id: db2Name});
+    assert.eq(null, db1EntryOriginal.version);
+    assert.eq(null, db2EntryOriginal.version);
+
+    // Ensure the shard does not have a cached entries in-memory or on-disk.
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db2Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db2Name, {});
+    checkOnDiskDatabaseVersion(st.shard0, db1Name, undefined);
+    checkOnDiskDatabaseVersion(st.shard0, db2Name, undefined);
+
+    // Create a collection in 'db1' to ensure 'db1' gets created on the primary shard.
+    // TODO (SERVER-34431): Remove this once the DatabaseShardingState is in a standalone map.
+    assert.commandWorked(st.s.getDB(db1Name).runCommand({create: collName}));
+
+    // Force the shard database to refresh and ensure it writes a cache entry but does not cache the
+    // version in memory.
+    assert.commandWorked(st.rs0.getPrimary().adminCommand({_flushDatabaseCacheUpdates: db1Name}));
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db2Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db2Name, {});
+    checkOnDiskDatabaseVersion(st.shard0, db1Name, db1EntryOriginal);
+    checkOnDiskDatabaseVersion(st.shard0, db2Name, undefined);
+
+    //
+    // setFCV 4.0 (upgrade)
+    //
+
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+
+    // Database versions should have been generated for the authoritative database entries.
+    const db1EntryFCV40 = st.s.getDB("config").getCollection("databases").findOne({_id: db1Name});
+    const db2EntryFCV40 = st.s.getDB("config").getCollection("databases").findOne({_id: db2Name});
+    assert.neq(null, db1EntryFCV40.version);
+    assert.neq(null, db2EntryFCV40.version);
+
+    // Before the shard receives a versioned request, its in-memory and on-disk caches should not
+    // have been refreshed.
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db2Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db2Name, {});
+    checkOnDiskDatabaseVersion(st.shard0, db1Name, db1EntryOriginal);
+    checkOnDiskDatabaseVersion(st.shard0, db2Name, undefined);
+
+    // After receiving a versioned request for a database, the shard should refresh its in-memory
+    // and on-disk caches for that database.
+
+    // This is needed because mongos will not automatically refresh its database entry after the
+    // cluster's FCV changes to pick up the database version (see SERVER-34460).
+    assert.commandWorked(st.s.adminCommand({flushRouterConfig: 1}));
+
+    assert.commandWorked(st.s.getDB(db1Name).runCommand(
+        {listCollections: 1, $readPreference: {mode: "secondary"}, readConcern: {level: "local"}}));
+
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db1Name, db1EntryFCV40.version);
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db1Name, db1EntryFCV40.version);
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db2Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db2Name, {});
+    checkOnDiskDatabaseVersion(st.shard0, db1Name, db1EntryFCV40);
+    checkOnDiskDatabaseVersion(st.shard0, db2Name, undefined);
+
+    //
+    // setFCV 3.6 (downgrade)
+    //
+
+    assert.commandWorked(st.s.adminCommand({setFeatureCompatibilityVersion: lastStableFCV}));
+
+    // Database versions should have been removed from the authoritative database entries.
+    const db1EntryFCV36 = st.s.getDB("config").getCollection("databases").findOne({_id: db1Name});
+    const db2EntryFCV36 = st.s.getDB("config").getCollection("databases").findOne({_id: db2Name});
+    assert.docEq(db1EntryOriginal, db1EntryFCV36);
+    assert.docEq(db2EntryOriginal, db2EntryFCV36);
+
+    // The shard's in-memory database cache should have been cleared, but its on-disk cache left
+    // untouched.
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db1Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getPrimary(), db2Name, {});
+    checkInMemoryDatabaseVersion(st.rs0.getSecondary(), db2Name, {});
+    checkOnDiskDatabaseVersion(st.shard0, db1Name, db1EntryFCV40);
+    checkOnDiskDatabaseVersion(st.shard0, db2Name, undefined);
+
+    st.stop();
+})();
diff --git a/src/mongo/db/commands/feature_compatibility_version.cpp b/src/mongo/db/commands/feature_compatibility_version.cpp
index 2216c24dd6d..932f8d29553 100644
--- a/src/mongo/db/commands/feature_compatibility_version.cpp
+++ b/src/mongo/db/commands/feature_compatibility_version.cpp
@@ -33,6 +33,7 @@
 #include "mongo/db/commands/feature_compatibility_version.h"
 
 #include "mongo/base/status.h"
+#include "mongo/db/catalog_raii.h"
 #include "mongo/db/commands/feature_compatibility_version_documentation.h"
 #include "mongo/db/commands/feature_compatibility_version_parser.h"
 #include "mongo/db/dbdirectclient.h"
@@ -41,6 +42,7 @@
 #include "mongo/db/operation_context.h"
 #include "mongo/db/repl/optime.h"
 #include "mongo/db/repl/storage_interface.h"
+#include "mongo/db/s/database_sharding_state.h"
 #include "mongo/db/server_parameters.h"
 #include "mongo/db/service_context.h"
 #include "mongo/db/storage/storage_engine.h"
@@ -154,12 +156,30 @@ void FeatureCompatibilityVersion::onInsertOrUpdate(OperationContext* opCtx, cons
               << FeatureCompatibilityVersionParser::toString(newVersion);
     }
 
-    // On commit, update the server parameters, close any connections with a wire version that is
-    // below the minimum, and abort any open transactions if downgrading.
     opCtx->recoveryUnit()->onCommit([opCtx, newVersion]() {
         serverGlobalParams.featureCompatibility.setVersion(newVersion);
         updateMinWireVersion();
 
+        if (newVersion == ServerGlobalParams::FeatureCompatibility::Version::kFullyDowngradedTo36) {
+            // Clear the in-memory cached database versions on downgrade, so that we do not check
+            // databaseVersion in FCV 3.6 (it would be meaningless, since databases do not have
+            // versions in FCV 3.6).
+            // TODO: Once SERVER-34431 goes in, just clear the DatabaseShardingStateMap.
+            std::vector<std::string> dbNames;
+            getGlobalServiceContext()->getGlobalStorageEngine()->listDatabases(&dbNames);
+            for (const auto& dbName : dbNames) {
+                if (dbName == "admin") {
+                    // The 'admin' database is already locked, since the FCV document is in
+                    // admin.system.version. Just skip 'admin', since it is not versioned.
+                    continue;
+                }
+                AutoGetDb autoDb(opCtx, dbName, MODE_X);
+                if (autoDb.getDb()) {
+                    DatabaseShardingState::get(autoDb.getDb()).setDbVersion(opCtx, boost::none);
+                }
+            }
+        }
+
         if (newVersion != ServerGlobalParams::FeatureCompatibility::Version::kFullyDowngradedTo36) {
             // Close all incoming connections from internal clients with binary versions lower than
             // ours.
diff --git a/src/mongo/db/s/SConscript b/src/mongo/db/s/SConscript
index 6c43720e075..0c3d7918cb6 100644
--- a/src/mongo/db/s/SConscript
+++ b/src/mongo/db/s/SConscript
@@ -55,6 +55,7 @@ env.Library(
         'split_vector.cpp',
     ],
     LIBDEPS=[
+        '$BUILD_DIR/mongo/db/commands/mongod_fcv',
         '$BUILD_DIR/mongo/db/db_raii',
         '$BUILD_DIR/mongo/db/dbhelpers',
         '$BUILD_DIR/mongo/db/repl/oplog',
diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
index e641d43578e..cc3e9533ecb 100644
--- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
+++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
@@ -34,6 +34,7 @@
 
 #include "mongo/db/catalog/database_holder.h"
 #include "mongo/db/catalog_raii.h"
+#include "mongo/db/commands/feature_compatibility_version.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/db/s/collection_sharding_state.h"
 #include "mongo/db/s/database_sharding_state.h"
@@ -200,13 +201,39 @@ void forceDatabaseRefresh(OperationContext* opCtx, const StringData dbName) {
         uassertStatusOK(Grid::get(opCtx)->catalogCache()->getDatabaseWithRefresh(opCtx, dbName))
             .databaseVersion();
 
-    // First, check under a shared lock if another thread already updated the cached version.
-    // This is a best-effort optimization to make as few threads as possible to convoy on the
-    // exclusive lock below.
-    {
-        // Take the DBLock directly rather than using AutoGetDb, to prevent a recursive call
-        // into checkDbVersion().
-        Lock::DBLock dbLock(opCtx, dbName, MODE_IS);
+    // Only set the in-memory version in FCV 4.0, and hold the lock across checking the FCV and
+    // setting the version.
+    Lock::SharedLock lk(opCtx->lockState(), FeatureCompatibilityVersion::fcvLock);
+    if (serverGlobalParams.featureCompatibility.getVersion() ==
+        ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo40) {
+        // First, check under a shared lock if another thread already updated the cached version.
+        // This is a best-effort optimization to make as few threads as possible to convoy on the
+        // exclusive lock below.
+        {
+            // Take the DBLock directly rather than using AutoGetDb, to prevent a recursive call
+            // into checkDbVersion().
+            Lock::DBLock dbLock(opCtx, dbName, MODE_IS);
+            const auto db = dbHolder().get(opCtx, dbName);
+            if (!db) {
+                log() << "Database " << dbName
+                      << " has been dropped; not caching the refreshed databaseVersion";
+                return;
+            }
+
+            const auto cachedDbVersion = DatabaseShardingState::get(db).getDbVersion(opCtx);
+            if (cachedDbVersion && refreshedDbVersion &&
+                cachedDbVersion->getUuid() == refreshedDbVersion->getUuid() &&
+                cachedDbVersion->getLastMod() >= refreshedDbVersion->getLastMod()) {
+                LOG(2) << "Skipping setting cached databaseVersion for " << dbName
+                       << " to refreshed version " << refreshedDbVersion->toBSON()
+                       << " because current cached databaseVersion is already "
+                       << cachedDbVersion->toBSON();
+                return;
+            }
+        }
+
+        // The cached version is older than the refreshed version; update the cached version.
+        Lock::DBLock dbLock(opCtx, dbName, MODE_X);
         const auto db = dbHolder().get(opCtx, dbName);
         if (!db) {
             log() << "Database " << dbName
@@ -214,28 +241,8 @@ void forceDatabaseRefresh(OperationContext* opCtx, const StringData dbName) {
             return;
         }
 
-        const auto cachedDbVersion = DatabaseShardingState::get(db).getDbVersion(opCtx);
-        if (cachedDbVersion && refreshedDbVersion &&
-            cachedDbVersion->getUuid() == refreshedDbVersion->getUuid() &&
-            cachedDbVersion->getLastMod() >= refreshedDbVersion->getLastMod()) {
-            LOG(2) << "Skipping setting cached databaseVersion for " << dbName
-                   << " to refreshed version " << refreshedDbVersion->toBSON()
-                   << " because current cached databaseVersion is already "
-                   << cachedDbVersion->toBSON();
-            return;
-        }
+        DatabaseShardingState::get(db).setDbVersion(opCtx, std::move(refreshedDbVersion));
     }
-
-    // The cached version is older than the refreshed version; update the cached version.
-    Lock::DBLock dbLock(opCtx, dbName, MODE_X);
-    const auto db = dbHolder().get(opCtx, dbName);
-    if (!db) {
-        log() << "Database " << dbName
-              << " has been dropped; not caching the refreshed databaseVersion";
-        return;
-    }
-
-    DatabaseShardingState::get(db).setDbVersion(opCtx, std::move(refreshedDbVersion));
 }
 
 }  // namespace mongo