SERVER-48739 Add cached max KeyString to ChunkInfo class

8 files changed, 51 insertions, 45 deletions

diff --git a/src/mongo/bson/ordering.h b/src/mongo/bson/ordering.h
index c1052e36a90..fe4de6d46b3 100644
--- a/src/mongo/bson/ordering.h
+++ b/src/mongo/bson/ordering.h
@@ -47,6 +47,11 @@ class Ordering {
 public:
     static constexpr size_t kMaxCompoundIndexKeys = size_t{32};
 
+    static const Ordering& allAscending() {
+        static Ordering allAscending{0};
+        return allAscending;
+    }
+
     Ordering(const Ordering& r) : bits(r.bits) {}
     void operator=(const Ordering& r) {
         bits = r.bits;
diff --git a/src/mongo/s/chunk.cpp b/src/mongo/s/chunk.cpp
index 29420ca766b..f224fbd8c87 100644
--- a/src/mongo/s/chunk.cpp
+++ b/src/mongo/s/chunk.cpp
@@ -41,6 +41,7 @@ namespace mongo {
 
 ChunkInfo::ChunkInfo(const ChunkType& from)
     : _range(from.getMin(), from.getMax()),
+      _maxKeyString(ShardKeyPattern::toKeyString(from.getMax())),
       _shardId(from.getShard()),
       _lastmod(from.getVersion()),
       _history(from.getHistory()),
diff --git a/src/mongo/s/chunk.h b/src/mongo/s/chunk.h
index 6c207cef7de..e238809d248 100644
--- a/src/mongo/s/chunk.h
+++ b/src/mongo/s/chunk.h
@@ -57,6 +57,10 @@ public:
         return _range.getMax();
     }
 
+    const std::string& getMaxKeyString() const {
+        return _maxKeyString;
+    }
+
     const ShardId& getShardIdAt(const boost::optional<Timestamp>& ts) const;
 
     /**
@@ -104,6 +108,7 @@ public:
 
 private:
     const ChunkRange _range;
+    const std::string _maxKeyString;
 
     const ShardId _shardId;
 
diff --git a/src/mongo/s/chunk_manager.cpp b/src/mongo/s/chunk_manager.cpp
index 6d4586d99a8..8a9b7263cad 100644
--- a/src/mongo/s/chunk_manager.cpp
+++ b/src/mongo/s/chunk_manager.cpp
@@ -67,16 +67,6 @@ void checkAllElementsAreOfType(BSONType type, const BSONObj& o) {
             allElementsAreOfType(type, o));
 }
 
-std::string extractKeyStringInternal(const BSONObj& shardKeyValue, Ordering ordering) {
-    BSONObjBuilder strippedKeyValue;
-    for (const auto& elem : shardKeyValue) {
-        strippedKeyValue.appendAs(elem, ""_sd);
-    }
-
-    KeyString::Builder ks(KeyString::Version::V1, strippedKeyValue.done(), ordering);
-    return {ks.getBuffer(), ks.getSize()};
-}
-
 }  // namespace
 
 ShardVersionMap ChunkMap::constructShardVersionMap(const OID& epoch) const {
@@ -123,22 +113,18 @@ ShardVersionMap ChunkMap::constructShardVersionMap(const OID& epoch) const {
         if (lastMax && !SimpleBSONObjComparator::kInstance.evaluate(*lastMax == rangeMin)) {
             if (SimpleBSONObjComparator::kInstance.evaluate(*lastMax < rangeMin))
                 uasserted(ErrorCodes::ConflictingOperationInProgress,
-                          str::stream()
-                              << "Gap exists in the routing table between chunks "
-                              << _chunkMap
-                                     .at(extractKeyStringInternal(*lastMax, _shardKeyOrdering))
-                                     ->getRange()
-                                     .toString()
-                              << " and " << rangeLast->second->getRange().toString());
+                          str::stream() << "Gap exists in the routing table between chunks "
+                                        << _chunkMap.at(ShardKeyPattern::toKeyString(*lastMax))
+                                               ->getRange()
+                                               .toString()
+                                        << " and " << rangeLast->second->getRange().toString());
             else
                 uasserted(ErrorCodes::ConflictingOperationInProgress,
-                          str::stream()
-                              << "Overlap exists in the routing table between chunks "
-                              << _chunkMap
-                                     .at(extractKeyStringInternal(*lastMax, _shardKeyOrdering))
-                                     ->getRange()
-                                     .toString()
-                              << " and " << rangeLast->second->getRange().toString());
+                          str::stream() << "Overlap exists in the routing table between chunks "
+                                        << _chunkMap.at(ShardKeyPattern::toKeyString(*lastMax))
+                                               ->getRange()
+                                               .toString()
+                                        << " and " << rangeLast->second->getRange().toString());
         }
 
         if (!firstMin)
@@ -164,8 +150,8 @@ ShardVersionMap ChunkMap::constructShardVersionMap(const OID& epoch) const {
 }
 
 void ChunkMap::addChunk(const ChunkType& chunk) {
-    const auto chunkMinKeyString = extractKeyStringInternal(chunk.getMin(), _shardKeyOrdering);
-    const auto chunkMaxKeyString = extractKeyStringInternal(chunk.getMax(), _shardKeyOrdering);
+    const auto chunkMinKeyString = ShardKeyPattern::toKeyString(chunk.getMin());
+    const auto chunkMaxKeyString = ShardKeyPattern::toKeyString(chunk.getMax());
 
     // Returns the first chunk with a max key that is > min - implies that the chunk overlaps
     // min
@@ -215,16 +201,15 @@ std::shared_ptr<ChunkInfo> ChunkMap::findIntersectingChunk(const BSONObj& shardK
 
 ChunkMap::ChunkInfoMap::const_iterator ChunkMap::_findIntersectingChunk(
     const BSONObj& shardKey) const {
-    return _chunkMap.upper_bound(extractKeyStringInternal(shardKey, _shardKeyOrdering));
+    return _chunkMap.upper_bound(ShardKeyPattern::toKeyString(shardKey));
 }
 
 std::pair<ChunkMap::ChunkInfoMap::const_iterator, ChunkMap::ChunkInfoMap::const_iterator>
 ChunkMap::_overlappingBounds(const BSONObj& min, const BSONObj& max, bool isMaxInclusive) const {
-    const auto itMin = _chunkMap.upper_bound(extractKeyStringInternal(min, _shardKeyOrdering));
+    const auto itMin = _chunkMap.upper_bound(ShardKeyPattern::toKeyString(min));
     const auto itMax = [&]() {
-        auto it = isMaxInclusive
-            ? _chunkMap.upper_bound(extractKeyStringInternal(max, _shardKeyOrdering))
-            : _chunkMap.lower_bound(extractKeyStringInternal(max, _shardKeyOrdering));
+        auto it = isMaxInclusive ? _chunkMap.upper_bound(ShardKeyPattern::toKeyString(max))
+                                 : _chunkMap.lower_bound(ShardKeyPattern::toKeyString(max));
         return it == _chunkMap.end() ? it : ++it;
     }();
 
@@ -641,13 +626,12 @@ std::shared_ptr<RoutingTableHistory> RoutingTableHistory::makeNew(
     bool unique,
     OID epoch,
     const std::vector<ChunkType>& chunks) {
-    auto ordering = Ordering::make(shardKeyPattern.toBSON());
     return RoutingTableHistory(std::move(nss),
                                std::move(uuid),
                                std::move(shardKeyPattern),
                                std::move(defaultCollator),
                                std::move(unique),
-                               {std::move(ordering)},
+                               ChunkMap{},
                                {0, 0, epoch})
         .makeUpdated(chunks);
 }
diff --git a/src/mongo/s/chunk_manager.h b/src/mongo/s/chunk_manager.h
index b91750c4073..dfd7d66cc4e 100644
--- a/src/mongo/s/chunk_manager.h
+++ b/src/mongo/s/chunk_manager.h
@@ -72,7 +72,7 @@ class ChunkMap {
     using ChunkInfoMap = std::map<std::string, std::shared_ptr<ChunkInfo>>;
 
 public:
-    ChunkMap(Ordering ordering) : _shardKeyOrdering(std::move(ordering)) {}
+    ChunkMap() {}
 
     size_t size() const {
         return _chunkMap.size();
@@ -111,7 +111,6 @@ private:
         const BSONObj& min, const BSONObj& max, bool isMaxInclusive) const;
 
     ChunkInfoMap _chunkMap;
-    const Ordering _shardKeyOrdering;
 };
 
 /**
@@ -260,11 +259,6 @@ private:
                         ChunkMap chunkMap,
                         ChunkVersion collectionVersion);
 
-    /**
-     * Does a single pass over the chunkMap and constructs the ShardVersionMap object.
-     */
-    ShardVersionMap _constructShardVersionMap() const;
-
     ChunkVersion _getVersion(const ShardId& shardName, bool throwOnStaleShard) const;
 
     // The shard versioning mechanism hinges on keeping track of the number of times we reload
diff --git a/src/mongo/s/chunk_map_test.cpp b/src/mongo/s/chunk_map_test.cpp
index a27fc59e889..96371906310 100644
--- a/src/mongo/s/chunk_map_test.cpp
+++ b/src/mongo/s/chunk_map_test.cpp
@@ -52,8 +52,6 @@ private:
 }  // namespace
 
 TEST_F(ChunkMapTest, TestAddChunk) {
-    ChunkMap chunkMap(Ordering::make(getShardKeyPattern().toBSON()));
-
     const OID epoch = OID::gen();
     ChunkVersion version{1, 0, epoch};
 
@@ -63,13 +61,14 @@ TEST_F(ChunkMapTest, TestAddChunk) {
                   version,
                   kThisShard};
 
+    ChunkMap chunkMap{};
     chunkMap.addChunk(chunk);
 
     ASSERT_EQ(chunkMap.size(), 1);
 }
 
 TEST_F(ChunkMapTest, TestEnumerateAllChunks) {
-    ChunkMap chunkMap(Ordering::make(getShardKeyPattern().toBSON()));
+    ChunkMap chunkMap{};
 
     const OID epoch = OID::gen();
     ChunkVersion version{1, 0, epoch};
@@ -98,7 +97,7 @@ TEST_F(ChunkMapTest, TestEnumerateAllChunks) {
 }
 
 TEST_F(ChunkMapTest, TestIntersectingChunk) {
-    ChunkMap chunkMap(Ordering::make(getShardKeyPattern().toBSON()));
+    ChunkMap chunkMap{};
 
     const OID epoch = OID::gen();
     ChunkVersion version{1, 0, epoch};
@@ -122,7 +121,7 @@ TEST_F(ChunkMapTest, TestIntersectingChunk) {
 }
 
 TEST_F(ChunkMapTest, TestEnumerateOverlappingChunks) {
-    ChunkMap chunkMap(Ordering::make(getShardKeyPattern().toBSON()));
+    ChunkMap chunkMap{};
 
     const OID epoch = OID::gen();
     ChunkVersion version{1, 0, epoch};
diff --git a/src/mongo/s/shard_key_pattern.cpp b/src/mongo/s/shard_key_pattern.cpp
index e290074c544..7fd4830afd1 100644
--- a/src/mongo/s/shard_key_pattern.cpp
+++ b/src/mongo/s/shard_key_pattern.cpp
@@ -253,6 +253,18 @@ std::string ShardKeyPattern::toString() const {
     return toBSON().toString();
 }
 
+std::string ShardKeyPattern::toKeyString(const BSONObj& shardKey) {
+    BSONObjBuilder strippedKeyValue;
+    for (const auto& elem : shardKey) {
+        strippedKeyValue.appendAs(elem, ""_sd);
+    }
+
+    KeyString::Builder ks(
+        KeyString::Version::V1, strippedKeyValue.done(), Ordering::allAscending());
+
+    return {ks.getBuffer(), ks.getSize()};
+}
+
 bool ShardKeyPattern::isShardKey(const BSONObj& shardKey) const {
     const auto& keyPatternBSON = _keyPattern.toBSON();
 
diff --git a/src/mongo/s/shard_key_pattern.h b/src/mongo/s/shard_key_pattern.h
index a9672f65ee7..ca5baac3ec3 100644
--- a/src/mongo/s/shard_key_pattern.h
+++ b/src/mongo/s/shard_key_pattern.h
@@ -124,6 +124,12 @@ public:
     std::string toString() const;
 
     /**
+     * Converts the passed in key pattern into a KeyString.
+     * Note: this function strips the field names when creating the KeyString.
+     */
+    static std::string toKeyString(const BSONObj& shardKey);
+
+    /**
      * Returns true if the provided document is a shard key - i.e. has the same fields as the
      * shard key pattern and valid shard key values.
      */