SERVER-46199 Hide the RoutingTableHistory detail from the ChunkManager

8 files changed, 41 insertions, 88 deletions

diff --git a/src/mongo/db/s/metadata_manager.cpp b/src/mongo/db/s/metadata_manager.cpp
index 22a0e23282d..127d1b4c776 100644
--- a/src/mongo/db/s/metadata_manager.cpp
+++ b/src/mongo/db/s/metadata_manager.cpp
@@ -136,10 +136,6 @@ std::shared_ptr<ScopedCollectionDescription::Impl> MetadataManager::getActiveMet
             lg, shared_from_this(), std::move(activeMetadataTracker));
     }
 
-    auto cm = activeMetadata->getChunkManager();
-    ChunkManager chunkManagerAtClusterTime = ChunkManager(
-        cm->dbPrimary(), cm->dbVersion(), cm->getRoutingHistory(), atClusterTime->asTimestamp());
-
     class MetadataAtTimestamp : public ScopedCollectionDescription::Impl {
     public:
         MetadataAtTimestamp(CollectionMetadata metadata) : _metadata(std::move(metadata)) {}
@@ -152,8 +148,9 @@ std::shared_ptr<ScopedCollectionDescription::Impl> MetadataManager::getActiveMet
         CollectionMetadata _metadata;
     };
 
-    return std::make_shared<MetadataAtTimestamp>(
-        CollectionMetadata(chunkManagerAtClusterTime, activeMetadata->shardId()));
+    return std::make_shared<MetadataAtTimestamp>(CollectionMetadata(
+        ChunkManager::makeAtTime(*activeMetadata->getChunkManager(), atClusterTime->asTimestamp()),
+        activeMetadata->shardId()));
 }
 
 size_t MetadataManager::numberOfMetadataSnapshots() const {
diff --git a/src/mongo/db/s/metadata_manager_test.cpp b/src/mongo/db/s/metadata_manager_test.cpp
index cff36ce4269..ea40d8dcb61 100644
--- a/src/mongo/db/s/metadata_manager_test.cpp
+++ b/src/mongo/db/s/metadata_manager_test.cpp
@@ -129,7 +129,7 @@ protected:
         splitChunks.emplace_back(
             kNss, ChunkRange(maxKey, chunkToSplit.getMax()), chunkVersion, kOtherShard);
 
-        auto rt = cm->getRoutingHistory()->makeUpdated(boost::none, splitChunks);
+        auto rt = cm->getRoutingTableHistory_ForTest().makeUpdated(boost::none, splitChunks);
 
         return CollectionMetadata(ChunkManager(cm->dbPrimary(), cm->dbVersion(), rt, boost::none),
                                   kThisShard);
@@ -150,7 +150,7 @@ protected:
         auto chunkVersion = cm->getVersion();
         chunkVersion.incMajor();
 
-        auto rt = cm->getRoutingHistory()->makeUpdated(
+        auto rt = cm->getRoutingTableHistory_ForTest().makeUpdated(
             boost::none, {ChunkType(kNss, ChunkRange(minKey, maxKey), chunkVersion, kOtherShard)});
 
         return CollectionMetadata(ChunkManager(cm->dbPrimary(), cm->dbVersion(), rt, boost::none),
diff --git a/src/mongo/db/s/op_observer_sharding_impl.cpp b/src/mongo/db/s/op_observer_sharding_impl.cpp
index 521c4bf7b4f..60e93d7f0a2 100644
--- a/src/mongo/db/s/op_observer_sharding_impl.cpp
+++ b/src/mongo/db/s/op_observer_sharding_impl.cpp
@@ -64,10 +64,9 @@ void assertIntersectingChunkHasNotMoved(OperationContext* opCtx,
     auto shardKey = metadata.getShardKeyPattern().extractShardKeyFromDoc(doc);
 
     // We can assume the simple collation because shard keys do not support non-simple collations.
-    auto cm = metadata.getChunkManager();
-    ChunkManager chunkManagerAtClusterTime(
-        cm->dbPrimary(), cm->dbVersion(), cm->getRoutingHistory(), atClusterTime->asTimestamp());
-    auto chunk = chunkManagerAtClusterTime.findIntersectingChunkWithSimpleCollation(shardKey);
+    auto cmAtTimeOfWrite =
+        ChunkManager::makeAtTime(*metadata.getChunkManager(), atClusterTime->asTimestamp());
+    auto chunk = cmAtTimeOfWrite.findIntersectingChunkWithSimpleCollation(shardKey);
 
     // Throws if the chunk has moved since the timestamp of the running transaction's atClusterTime
     // read concern parameter.
diff --git a/src/mongo/s/catalog_cache.cpp b/src/mongo/s/catalog_cache.cpp
index a3d87fc598c..52a83f2c371 100644
--- a/src/mongo/s/catalog_cache.cpp
+++ b/src/mongo/s/catalog_cache.cpp
@@ -106,10 +106,15 @@ std::shared_ptr<RoutingTableHistory> refreshCollectionRoutingInfo(
         // Otherwise, we're making a whole new routing table.
         if (existingRoutingInfo &&
             existingRoutingInfo->getVersion().epoch() == collectionAndChunks.epoch) {
+            if (collectionAndChunks.changedChunks.size() == 1 &&
+                collectionAndChunks.changedChunks[0].getVersion() ==
+                    existingRoutingInfo->getVersion())
+                return existingRoutingInfo;
 
             return existingRoutingInfo->makeUpdated(std::move(collectionAndChunks.reshardingFields),
                                                     collectionAndChunks.changedChunks);
         }
+
         auto defaultCollator = [&]() -> std::unique_ptr<CollatorInterface> {
             if (!collectionAndChunks.defaultCollation.isEmpty()) {
                 // The collation should have been validated upon collection creation
@@ -118,6 +123,7 @@ std::shared_ptr<RoutingTableHistory> refreshCollectionRoutingInfo(
             }
             return nullptr;
         }();
+
         return RoutingTableHistory::makeNew(nss,
                                             collectionAndChunks.uuid,
                                             KeyPattern(collectionAndChunks.shardKeyPattern),
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index bb1dd46caec..edf6c8fc91c 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -211,7 +211,8 @@ void validateChunk(const std::shared_ptr<ChunkInfo>& chunk, const ChunkVersion&
     invariant(chunk->getLastmod() >= version);
 }
 
-ChunkMap ChunkMap::createMerged(const std::vector<std::shared_ptr<ChunkInfo>>& changedChunks) {
+ChunkMap ChunkMap::createMerged(
+    const std::vector<std::shared_ptr<ChunkInfo>>& changedChunks) const {
     size_t chunkMapIndex = 0;
     size_t changedChunkIndex = 0;
 
@@ -648,6 +649,10 @@ IndexBounds ChunkManager::collapseQuerySolution(const QuerySolutionNode* node) {
     return bounds;
 }
 
+ChunkManager ChunkManager::makeAtTime(const ChunkManager& cm, Timestamp clusterTime) {
+    return ChunkManager(cm.dbPrimary(), cm.dbVersion(), cm._rt, clusterTime);
+}
+
 std::string ChunkManager::toString() const {
     return _rt ? _rt->toString() : "UNSHARDED";
 }
@@ -724,27 +729,13 @@ std::shared_ptr<RoutingTableHistory> RoutingTableHistory::makeNew(
 
 std::shared_ptr<RoutingTableHistory> RoutingTableHistory::makeUpdated(
     boost::optional<TypeCollectionReshardingFields> reshardingFields,
-    const std::vector<ChunkType>& changedChunks) {
-
-    // It's possible for there to be one chunk in changedChunks without the routing table having
-    // changed. We skip copying the ChunkMap when this happens.
-    if (changedChunks.size() == 1 && changedChunks[0].getVersion() == getVersion()) {
-        return shared_from_this();
-    }
-
+    const std::vector<ChunkType>& changedChunks) const {
     auto changedChunkInfos = flatten(changedChunks);
     auto chunkMap = _chunkMap.createMerged(changedChunkInfos);
 
-    // If at least one diff was applied, the metadata is correct, but it might not have changed so
-    // in this case there is no need to recreate the chunk manager.
-    //
-    // NOTE: In addition to the above statement, it is also important that we return the same chunk
-    // manager object, because the write commands' code relies on changes of the chunk manager's
-    // sequence number to detect batch writes not making progress because of chunks moving across
-    // shards too frequently.
-    if (chunkMap.getVersion() == getVersion()) {
-        return shared_from_this();
-    }
+    // If at least one diff was applied, the collection's version must have advanced
+    invariant(getVersion().epoch() == chunkMap.getVersion().epoch());
+    invariant(getVersion().isOlderThan(chunkMap.getVersion()));
 
     return std::shared_ptr<RoutingTableHistory>(
         new RoutingTableHistory(_nss,
diff --git a/src/mongo/s/chunk_manager.h b/src/mongo/s/chunk_manager.h
index 0a2270c7b77..22330aeecfb 100644
--- a/src/mongo/s/chunk_manager.h
+++ b/src/mongo/s/chunk_manager.h
@@ -113,7 +113,8 @@ public:
     std::shared_ptr<ChunkInfo> findIntersectingChunk(const BSONObj& shardKey) const;
 
     void appendChunk(const std::shared_ptr<ChunkInfo>& chunk);
-    ChunkMap createMerged(const std::vector<std::shared_ptr<ChunkInfo>>& changedChunks);
+
+    ChunkMap createMerged(const std::vector<std::shared_ptr<ChunkInfo>>& changedChunks) const;
 
     BSONObj toBSON() const;
 
@@ -133,7 +134,7 @@ private:
  * In-memory representation of the routing table for a single sharded collection at various points
  * in time.
  */
-class RoutingTableHistory : public std::enable_shared_from_this<RoutingTableHistory> {
+class RoutingTableHistory {
     RoutingTableHistory(const RoutingTableHistory&) = delete;
     RoutingTableHistory& operator=(const RoutingTableHistory&) = delete;
 
@@ -179,7 +180,7 @@ public:
      */
     std::shared_ptr<RoutingTableHistory> makeUpdated(
         boost::optional<TypeCollectionReshardingFields> reshardingFields,
-        const std::vector<ChunkType>& changedChunks);
+        const std::vector<ChunkType>& changedChunks) const;
 
     const NamespaceString& nss() const {
         return _nss;
@@ -490,6 +491,12 @@ public:
     static IndexBounds collapseQuerySolution(const QuerySolutionNode* node);
 
     /**
+     * Constructs a new ChunkManager, which is a view of the underlying routing table at a different
+     * `clusterTime`.
+     */
+    static ChunkManager makeAtTime(const ChunkManager& cm, Timestamp clusterTime);
+
+    /**
      * Returns true if, for this shard, the chunks are identical in both chunk managers
      */
     bool compatibleWith(const ChunkManager& other, const ShardId& shard) const {
@@ -500,10 +507,6 @@ public:
         return _rt->uuidMatches(uuid);
     }
 
-    auto getRoutingHistory() const {
-        return _rt;
-    }
-
     boost::optional<UUID> getUUID() const {
         return _rt->getUUID();
     }
@@ -512,6 +515,10 @@ public:
         return _rt->getReshardingFields();
     }
 
+    const RoutingTableHistory& getRoutingTableHistory_ForTest() const {
+        return *_rt;
+    }
+
 private:
     ShardId _dbPrimary;
     DatabaseVersion _dbVersion;
diff --git a/src/mongo/s/chunk_manager_refresh_bm.cpp b/src/mongo/s/chunk_manager_refresh_bm.cpp
index 69a71e221e2..dbd0248f112 100644
--- a/src/mongo/s/chunk_manager_refresh_bm.cpp
+++ b/src/mongo/s/chunk_manager_refresh_bm.cpp
@@ -102,7 +102,9 @@ MONGO_COMPILER_NOINLINE auto makeChunkManagerWithOptimalBalancedDistribution(int
 
 MONGO_COMPILER_NOINLINE auto runIncrementalUpdate(const CollectionMetadata& cm,
                                                   const std::vector<ChunkType>& newChunks) {
-    auto rt = cm.getChunkManager()->getRoutingHistory()->makeUpdated(boost::none, newChunks);
+    auto rt =
+        cm.getChunkManager()->getRoutingTableHistory_ForTest().makeUpdated(boost::none, newChunks);
+
     return std::make_unique<CollectionMetadata>(
         ChunkManager(ShardId("shard0"), DatabaseVersion(UUID::gen(), 1), rt, boost::none),
         ShardId("shard0"));
diff --git a/src/mongo/s/routing_table_history_test.cpp b/src/mongo/s/routing_table_history_test.cpp
index 7e9b501fdce..6e42767fc69 100644
--- a/src/mongo/s/routing_table_history_test.cpp
+++ b/src/mongo/s/routing_table_history_test.cpp
@@ -361,55 +361,6 @@ TEST_F(RoutingTableHistoryTest, TestSplits) {
     ASSERT_EQ(v2, rt2->getVersion(kThisShard));
 }
 
-TEST_F(RoutingTableHistoryTest, TestReplaceChunk) {
-    const OID epoch = OID::gen();
-    ChunkVersion version{2, 2, epoch};
-
-    std::vector<ChunkType> initialChunks = {
-        ChunkType{kNss,
-                  ChunkRange{getShardKeyPattern().globalMin(), BSON("a" << 0)},
-                  ChunkVersion{2, 1, epoch},
-                  kThisShard},
-        ChunkType{kNss,
-                  ChunkRange{BSON("a" << 0), getShardKeyPattern().globalMax()},
-                  ChunkVersion{2, 2, epoch},
-                  kThisShard}};
-
-    auto rt = RoutingTableHistory::makeNew(kNss,
-                                           UUID::gen(),
-                                           getShardKeyPattern(),
-                                           nullptr,
-                                           false,
-                                           epoch,
-                                           boost::none,
-                                           {initialChunks});
-
-    std::vector<ChunkType> changedChunks = {
-        ChunkType{kNss,
-                  ChunkRange{BSON("a" << 0), getShardKeyPattern().globalMax()},
-                  ChunkVersion{2, 2, epoch},
-                  kThisShard}};
-
-    auto rt1 = rt->makeUpdated(boost::none, changedChunks);
-    auto v1 = ChunkVersion{2, 2, epoch};
-    ASSERT_EQ(v1, rt1->getVersion(kThisShard));
-    ASSERT_EQ(rt1->numChunks(), 2);
-    ASSERT_EQ(rt.get(), rt1.get());
-
-    std::shared_ptr<ChunkInfo> found;
-
-    rt1->forEachChunk(
-        [&](auto& chunkInfo) {
-            if (chunkInfo->getShardIdAt(boost::none) == kThisShard) {
-                found = chunkInfo;
-                return false;
-            }
-            return true;
-        },
-        BSON("a" << 0));
-    ASSERT(found);
-}
-
 TEST_F(RoutingTableHistoryTest, TestReplaceEmptyChunk) {
     const OID epoch = OID::gen();