SERVER-99277 Avoid full refresh of sharding metadata on shard removal (#31891)

GitOrigin-RevId: fdbf6abdf2a141f250c65017adc75a61b4a0dd72

Author: afuschetto

etc/backports_required_for_multiversion_tests.yml

@@ -579,6 +579,8 @@ last-continuous:
     ticket: SERVER-100489
   - test_file: jstests/sharding/sharded_data_distribution.js
     ticket: SERVER-88400
+  - test_file: jstests/sharding/metadata_refresh_on_shard_removal.js
+    ticket: SERVER-99277
   suites: null
 last-lts:
   all:
@@ -1210,4 +1212,6 @@ last-lts:
     ticket: SERVER-100489
   - test_file: jstests/sharding/sharded_data_distribution.js
     ticket: SERVER-88400
+  - test_file: jstests/sharding/metadata_refresh_on_shard_removal.js
+    ticket: SERVER-99277
   suites: null

jstests/sharding/shard_removal_triggers_catalog_cache_invalidation.js renamed to jstests/sharding/metadata_refresh_on_shard_removal.js

@@ -28,23 +28,30 @@ const dbName = 'TestDB';
  * 4. Remove shard0 by sending removeShard through router 0. All data will be migrated to shard1.
  * 5. Send a query through router 1 to target the sharded collection. This should correctly target
  *    shard1.
+ * 6. Ensure that no full refresh of the routing information is triggered on router 1, as a result
+ *    of the CRUD operations on the sharded collection after the shard0 removal.
  */
 (() => {
     jsTestLog(
-        "Test that sharded collections with data on a shard that gets removed are correctly invalidated in a router's catalog cache.");
+        "Test that metadata of a sharded collection with data on a shard that gets removed is correctly invalidated in a router's catalog cache.");
 
     const shardedCollName = 'Coll';
     const shardedCollNs = dbName + '.' + shardedCollName;
 
-    var st = new ShardingTest({shards: 2, mongos: 2});
+    var st = new ShardingTest({
+        shards: 2,
+        mongos: 2,
+        mongosOptions: {setParameter: {noCatalogCachePurgeOnShardRemoval: true}}
+    });
+
     let router0ShardedColl = st.s0.getDB(dbName)[shardedCollName];
     let router1ShardedColl = st.s1.getDB(dbName)[shardedCollName];
 
     assert.commandWorked(st.s0.adminCommand({enableSharding: dbName}));
     st.ensurePrimaryShard(dbName, st.shard1.shardName);
     assert.commandWorked(st.s0.adminCommand({shardCollection: shardedCollNs, key: {_id: 1}}));
 
-    // Make sure data is inserted into shard0
+    // Make sure data is inserted into shard0.
     assert.commandWorked(st.s0.adminCommand({
         moveChunk: shardedCollNs,
         find: {_id: -1},
@@ -75,6 +82,9 @@ const dbName = 'TestDB';
         st.rs0.stopSet();
     }
 
+    const initCatalogCacheStats =
+        st.s1.getDB(dbName).serverStatus({}).shardingStatistics.catalogCache;
+
     // Ensure that s1, the router which did not run removeShard, eventually stops targeting chunks
     // for the sharded collection which previously resided on a shard that no longer exists.
     assert.soon(() => {
@@ -87,6 +97,18 @@ const dbName = 'TestDB';
         }
     });
 
+    // Removing shard0 updates the metadata version in the catalog cache of all nodes. The
+    // subsequent CRUD operation on router1 triggered a incremental refresh (instead of a full
+    // refresh) of its catalog cache.
+    const updatedCatalogCacheStats =
+        st.s1.getDB(dbName).serverStatus({}).shardingStatistics.catalogCache;
+    assert.eq(0,
+              updatedCatalogCacheStats.countFullRefreshesStarted -
+                  initCatalogCacheStats.countFullRefreshesStarted);
+    assert.eq(1,
+              updatedCatalogCacheStats.countIncrementalRefreshesStarted -
+                  initCatalogCacheStats.countIncrementalRefreshesStarted);
+
     st.stop();
 })();
 
@@ -99,8 +121,10 @@ const dbName = 'TestDB';
  * 3. Ensure both routers have up-to-date routing info.
  * 4. movePrimary for the database to shard1.
  * 4. Remove shard0 by sending removeShard through router 0.
- * 5. Send a query through router 1 to target the sharded and unsharded collections. This should
- *    correctly target shard1.
+ * 5. Send a query through router 1 to target the unsharded collection. This should correctly target
+ *    shard1.
+ * 6. Ensure that no full refresh of the routing information is triggered on router 1, as a result
+ *    of the CRUD operations on the unsharded collection after the shard0 removal.
  */
 (() => {
     jsTestLog(
@@ -152,6 +176,13 @@ const dbName = 'TestDB';
             return false;
         }
     });
+
+    // TODO (SERVER-80724): Implement the following test case when database metadata refresh
+    // statistics are available.
+    // Removing shard0 updates the metadata version in the catalog cache of all nodes. The
+    // subsequent CRUD operation on router1 triggered a incremental refresh (instead of a full
+    // refresh) of its catalog cache.
+
     st.stop();
 })();
 })();

src/mongo/db/s/README.md

@@ -111,7 +111,6 @@ collection or database. A full refresh occurs when:
 Methods that will mark routing table cache information as stale (sharded collection).
 
 * [invalidateShardOrEntireCollectionEntryForShardedCollection](https://github.com/mongodb/mongo/blob/62d9485657717bf61fbb870cb3d09b52b1a614dd/src/mongo/s/catalog_cache.h#L226-L236)
-* [invalidateEntriesThatReferenceShard](https://github.com/mongodb/mongo/blob/62d9485657717bf61fbb870cb3d09b52b1a614dd/src/mongo/s/catalog_cache.h#L270-L274)
 * [invalidateCollectionEntry_LINEARIZABLE](https://github.com/mongodb/mongo/blob/32fe49396dec58836033bca67ad1360b1a80f03c/src/mongo/s/catalog_cache.h#L211-L216)
 
 Methods that will mark routing table cache information as stale (database).

src/mongo/db/s/sharding_initialization_mongod.cpp

@@ -67,6 +67,7 @@
 #include "mongo/s/client/sharding_connection_hook.h"
 #include "mongo/s/config_server_catalog_cache_loader.h"
 #include "mongo/s/grid.h"
+#include "mongo/s/mongod_and_mongos_server_parameters_gen.h"
 
 #define MONGO_LOGV2_DEFAULT_COMPONENT ::mongo::logv2::LogComponent::kSharding
 
@@ -641,11 +642,14 @@ void initializeGlobalShardingStateForMongoD(OperationContext* opCtx,
     // List of hooks which will be called by the ShardRegistry when it discovers a shard has been
     // removed.
     std::vector<ShardRegistry::ShardRemovalHook> shardRemovalHooks = {
-        // Invalidate appropriate entries in the CatalogCache when a shard is removed. It's safe to
-        // capture the CatalogCache pointer since the Grid (and therefore CatalogCache and
-        // ShardRegistry) are never destroyed.
+        // It's safe to capture the CatalogCache pointer since the Grid (and therefore CatalogCache
+        // and ShardRegistry) are never destroyed.
         [catCache = catalogCache.get()](const ShardId& removedShard) {
-            catCache->invalidateEntriesThatReferenceShard(removedShard);
+            if (gNoCatalogCachePurgeOnShardRemoval) {
+                catCache->advanceTimeInStoreForEntriesThatReferenceShard(removedShard);
+            } else {
+                catCache->invalidateEntriesThatReferenceShard(removedShard);
+            }
         }};
 
     auto shardRegistry = std::make_unique<ShardRegistry>(

src/mongo/s/catalog_cache.cpp

@@ -704,6 +704,56 @@ void CatalogCache::invalidateShardOrEntireCollectionEntryForShardedCollection(
     }
 }
 
+void CatalogCache::advanceTimeInStoreForEntriesThatReferenceShard(const ShardId& shardId) {
+    LOGV2_DEBUG(
+        9927700,
+        1,
+        "Advancing the cached version for databases and collections referencing a specific shard",
+        "shardId"_attr = shardId);
+
+    const auto dbEntries = _databaseCache.peekLatestCachedIf(
+        [&](const std::string&, const DatabaseType& dbt) { return dbt.getPrimary() == shardId; });
+    for (const auto& dbEntry : dbEntries) {
+        LOGV2_DEBUG(9927701,
+                    1,
+                    "Advancing the cached version for a database",
+                    "db"_attr = dbEntry->getName());
+
+        _databaseCache.advanceTimeInStore(
+            dbEntry->getName(),
+            ComparableDatabaseVersion::makeComparableDatabaseVersionForForcedRefresh());
+    }
+
+    const auto collEntries = _collectionCache.peekLatestCachedIf(
+        [&](const NamespaceString&, const OptionalRoutingTableHistory& ort) {
+            if (!ort.optRt) {
+                return false;
+            }
+            const auto& rt = *ort.optRt;
+
+            std::set<ShardId> shardIds;
+            rt.getAllShardIds(&shardIds);
+
+            return shardIds.find(shardId) != shardIds.end();
+        });
+    for (const auto& collEntry : collEntries) {
+        invariant(collEntry->optRt);
+        const auto& rt = *collEntry->optRt;
+
+        LOGV2_DEBUG(9927702,
+                    1,
+                    "Advancing the cached version for a collection",
+                    "namespace"_attr = rt.nss());
+
+        _collectionCache.advanceTimeInStore(
+            rt.nss(), ComparableChunkVersion::makeComparableChunkVersionForForcedRefresh());
+    }
+
+    LOGV2(9927703,
+          "Advanced the cached version for databases and collections referencing a specific shard",
+          "shardId"_attr = shardId);
+}
+
 void CatalogCache::invalidateEntriesThatReferenceShard(const ShardId& shardId) {
     LOGV2_DEBUG(4997600,
                 1,

src/mongo/s/catalog_cache.h

@@ -272,6 +272,13 @@ class CatalogCache {
         const boost::optional<ShardVersion>& wantedVersion,
         const ShardId& shardId);
 
+    /**
+     * Notifies the cache that there is a (possibly) newer version on the backing store for all the
+     * entries that reference the passed shard. This will trigger an incremental refresh on the next
+     * cache access.
+     */
+    void advanceTimeInStoreForEntriesThatReferenceShard(const ShardId& shardId);
+
     /**
      * Non-blocking method, which invalidates all namespaces which contain data on the specified
      * shard and all databases which have the shard listed as their primary shard.

src/mongo/s/catalog_cache_test.cpp

@@ -261,20 +261,56 @@ TEST_F(CatalogCacheTest, GetDatabaseDrop) {
     ASSERT_EQUALS(ErrorCodes::NamespaceNotFound, swDatabase.getStatus());
 }
 
-TEST_F(CatalogCacheTest, InvalidateSingleDbOnShardRemoval) {
+TEST_F(CatalogCacheTest, AdvanceTimeInStoreForSingleDbOnShardRemoval) {
+    // Put initial metadata in the local cache.
     const auto dbName = "testDB";
     const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1));
     loadDatabases({DatabaseType(dbName, kShards[0], dbVersion)});
 
-    _catalogCache->invalidateEntriesThatReferenceShard(kShards[0]);
+    _catalogCache->advanceTimeInStoreForEntriesThatReferenceShard(kShards[0]);
+
+    // Put the new metadata in the loader.
     _catalogCacheLoader->setDatabaseRefreshReturnValue(DatabaseType(dbName, kShards[1], dbVersion));
+
+    // Refresh the local cache.
     const auto swDatabase = _catalogCache->getDatabase(operationContext(), dbName);
 
     ASSERT_OK(swDatabase.getStatus());
-    auto cachedDb = swDatabase.getValue();
+    const auto cachedDb = swDatabase.getValue();
     ASSERT_EQ(cachedDb->getPrimary(), kShards[1]);
 }
 
+TEST_F(CatalogCacheTest, AdvanceTimeInStoreForSingleCollectionOnShardRemoval) {
+    // Put initial metadata in the local cache.
+    const auto initDbVer = DatabaseVersion(UUID::gen(), Timestamp(1, 1));
+    const auto initDbEntry = DatabaseType(kNss.db().toString(), kShards[0], initDbVer);
+    loadDatabases({initDbEntry});
+    const auto initCollVer =
+        ShardVersionFactory::make(ChunkVersion({OID::gen(), Timestamp(1, 1)}, {1, 0}),
+                                  boost::optional<CollectionIndexes>(boost::none));
+    loadCollection(initCollVer);
+
+    _catalogCache->advanceTimeInStoreForEntriesThatReferenceShard(kShards[0]);
+
+    // Put the new metadata in the loader.
+    const auto newDbEntry = initDbEntry;
+    auto newCollVer = initCollVer;
+    newCollVer.placementVersion().incMajor();
+    const auto newCollEntry = makeCollectionType(newCollVer);
+    const auto newChunkEntries = makeChunks(newCollVer.placementVersion());
+
+    _catalogCacheLoader->setDatabaseRefreshReturnValue(newDbEntry);
+    _catalogCacheLoader->setCollectionRefreshReturnValue(newCollEntry);
+    _catalogCacheLoader->setChunkRefreshReturnValue(newChunkEntries);
+
+    // Refresh the local cache.
+    const auto swColl = _catalogCache->getCollectionRoutingInfo(operationContext(), kNss);
+
+    ASSERT_OK(swColl.getStatus());
+    auto cachedColl = swColl.getValue();
+    ASSERT_EQ(newCollVer, cachedColl.getCollectionVersion());
+}
+
 TEST_F(CatalogCacheTest, OnStaleDatabaseVersionNoVersion) {
     // onStaleDatabaseVesrsion must invalidate the database entry if invoked with no version
     const auto dbVersion = DatabaseVersion(UUID::gen(), Timestamp(1, 1));

src/mongo/s/mongod_and_mongos_server_parameters.idl

@@ -79,3 +79,13 @@ server_parameters:
     default: 500
     validator:
       gte: 0
+
+  noCatalogCachePurgeOnShardRemoval:
+    description: >-
+        Advances the catalog cache's time in store when a shard is removed, instead of purging the
+        cache content. This triggers an incremental refresh of the metadata on the next cache access
+        rather than a full (and more expensive) refresh.
+    set_at: [ startup ]
+    cpp_vartype: bool
+    cpp_varname: "gNoCatalogCachePurgeOnShardRemoval"
+    default: false

src/mongo/s/mongos_main.cpp

@@ -81,6 +81,7 @@
 #include "mongo/s/grid.h"
 #include "mongo/s/is_mongos.h"
 #include "mongo/s/load_balancer_support.h"
+#include "mongo/s/mongod_and_mongos_server_parameters_gen.h"
 #include "mongo/s/mongos_options.h"
 #include "mongo/s/mongos_server_parameters_gen.h"
 #include "mongo/s/mongos_topology_coordinator.h"
@@ -431,11 +432,14 @@ Status initializeSharding(OperationContext* opCtx) {
     // List of hooks which will be called by the ShardRegistry when it discovers a shard has been
     // removed.
     std::vector<ShardRegistry::ShardRemovalHook> shardRemovalHooks = {
-        // Invalidate appropriate entries in the catalog cache when a shard is removed. It's safe to
-        // capture the catalog cache pointer since the Grid (and therefore CatalogCache and
-        // ShardRegistry) are never destroyed.
+        // It's safe to capture the CatalogCache pointer since the Grid (and therefore CatalogCache
+        // and ShardRegistry) are never destroyed.
         [catCache = catalogCache.get()](const ShardId& removedShard) {
-            catCache->invalidateEntriesThatReferenceShard(removedShard);
+            if (gNoCatalogCachePurgeOnShardRemoval) {
+                catCache->advanceTimeInStoreForEntriesThatReferenceShard(removedShard);
+            } else {
+                catCache->invalidateEntriesThatReferenceShard(removedShard);
+            }
         }};
 
     if (!mongosGlobalParams.configdbs) {