SERVER-36443 Clear ChunkManager objects in metadata when no longer in use

(cherry picked from commit da8c0d18e7ba69ef2ce31236d34816f6fbe8cec3)

3 files changed, 134 insertions, 14 deletions

diff --git a/src/mongo/db/s/metadata_manager.cpp b/src/mongo/db/s/metadata_manager.cpp
index e1771c7b70b..f4d9bc1f13f 100644
--- a/src/mongo/db/s/metadata_manager.cpp
+++ b/src/mongo/db/s/metadata_manager.cpp
@@ -192,8 +192,11 @@ public:
         }
     }
 
-    const CollectionMetadata& get() override {
-        return _metadataTracker->metadata;
+    // This will only ever refer to the active metadata, so CollectionMetadata should never be
+    // boost::none
+    const CollectionMetadata& get() {
+        invariant(_metadataTracker->metadata);
+        return _metadataTracker->metadata.get();
     }
 
 private:
@@ -246,7 +249,8 @@ ScopedCollectionMetadata MetadataManager::getActiveMetadata(
         return {std::make_shared<RangePreserver>(lg, std::move(self), std::move(metadataTracker))};
     }
 
-    auto chunkManager = metadataTracker->metadata.getChunkManager();
+    auto chunkManager = metadataTracker->metadata->getChunkManager();
+
     auto chunkManagerAtClusterTime = std::make_shared<ChunkManager>(
         chunkManager->getRoutingHistory(), atClusterTime->asTimestamp());
 
@@ -263,7 +267,7 @@ ScopedCollectionMetadata MetadataManager::getActiveMetadata(
     };
 
     return {std::make_shared<MetadataAtTimestamp>(
-        CollectionMetadata(chunkManagerAtClusterTime, metadataTracker->metadata.shardId()))};
+        CollectionMetadata(chunkManagerAtClusterTime, metadataTracker->metadata->shardId()))};
 }
 
 size_t MetadataManager::numberOfMetadataSnapshots() const {
@@ -274,6 +278,18 @@ size_t MetadataManager::numberOfMetadataSnapshots() const {
     return _metadata.size() - 1;
 }
 
+int MetadataManager::numberOfEmptyMetadataSnapshots() const {
+    stdx::lock_guard<stdx::mutex> lg(_managerLock);
+
+    int emptyMetadataSnapshots = 0;
+    for (const auto& collMetadataTracker : _metadata) {
+        if (!collMetadataTracker->metadata)
+            emptyMetadataSnapshots++;
+    }
+
+    return emptyMetadataSnapshots;
+}
+
 void MetadataManager::refreshActiveMetadata(std::unique_ptr<CollectionMetadata> remoteMetadata) {
     stdx::lock_guard<stdx::mutex> lg(_managerLock);
 
@@ -289,7 +305,7 @@ void MetadataManager::refreshActiveMetadata(std::unique_ptr<CollectionMetadata>
     // Collection is becoming unsharded
     if (!remoteMetadata) {
         log() << "Marking collection " << _nss.ns() << " with "
-              << redact(_metadata.back()->metadata.toStringBasic()) << " as unsharded";
+              << redact(_metadata.back()->metadata->toStringBasic()) << " as unsharded";
 
         _receivingChunks.clear();
         _clearAllCleanups(lg);
@@ -308,7 +324,7 @@ void MetadataManager::refreshActiveMetadata(std::unique_ptr<CollectionMetadata>
         return;
     }
 
-    auto* const activeMetadata = &_metadata.back()->metadata;
+    auto* const activeMetadata = &_metadata.back()->metadata.get();
 
     // If the metadata being installed has a different epoch from ours, this means the collection
     // was dropped and recreated, so we must entirely reset the metadata state
@@ -360,17 +376,33 @@ void MetadataManager::_setActiveMetadata(WithLock wl, CollectionMetadata newMeta
 }
 
 void MetadataManager::_retireExpiredMetadata(WithLock lock) {
+    // Remove entries and schedule orphans for deletion only from the front of _metadata. We cannot
+    // remove an entry from the middle of _metadata because a previous entry (whose usageCount is
+    // not 0) could have a query that is actually still accessing those documents.
     while (_metadata.size() > 1 && !_metadata.front()->usageCounter) {
         if (!_metadata.front()->orphans.empty()) {
             log() << "Queries possibly dependent on " << _nss.ns()
                   << " range(s) finished; scheduling ranges for deletion";
-            // It is safe to push orphan ranges from _metadata.back(), even though new queries might
-            // start any time, because any request to delete a range it maps is rejected.
+
             _pushListToClean(lock, std::move(_metadata.front()->orphans));
         }
 
         _metadata.pop_front();
     }
+
+    // To avoid memory build up of ChunkManager objects, we can clear the CollectionMetadata object
+    // in an entry when its usageCount is 0 as long as it is not the last item in _metadata (which
+    // is the active metadata). If _metadata is empty, decrementing iter will be out of bounds, so
+    // we must check that the size is > 1 as well.
+    if (_metadata.size() > 1) {
+        auto iter = _metadata.begin();
+        while (iter != (--_metadata.end())) {
+            if ((*iter)->usageCounter == 0) {
+                (*iter)->metadata = boost::none;
+            }
+            ++iter;
+        }
+    }
 }
 
 void MetadataManager::toBSONPending(BSONArrayBuilder& bb) const {
@@ -403,7 +435,7 @@ void MetadataManager::append(BSONObjBuilder* builder) const {
     }
 
     BSONArrayBuilder amrArr(builder->subarrayStart("activeMetadataRanges"));
-    for (const auto& entry : _metadata.back()->metadata.getChunks()) {
+    for (const auto& entry : _metadata.back()->metadata->getChunks()) {
         BSONObjBuilder obj;
         ChunkRange r = ChunkRange(entry.first, entry.second);
         r.append(&obj);
@@ -425,7 +457,7 @@ void MetadataManager::_pushListToClean(WithLock, std::list<Deletion> ranges) {
     auto when = _rangesToClean.add(std::move(ranges));
     if (when) {
         scheduleCleanup(
-            _executor, _nss, _metadata.back()->metadata.getCollVersion().epoch(), *when);
+            _executor, _nss, _metadata.back()->metadata->getCollVersion().epoch(), *when);
     }
     invariant(ranges.empty());
 }
@@ -532,14 +564,15 @@ auto MetadataManager::_findNewestOverlappingMetadata(WithLock, ChunkRange const&
     invariant(!_metadata.empty());
 
     auto it = _metadata.rbegin();
-    if ((*it)->metadata.rangeOverlapsChunk(range)) {
+    if ((*it)->metadata && (*it)->metadata->rangeOverlapsChunk(range)) {
         return (*it).get();
     }
 
     ++it;
     for (; it != _metadata.rend(); ++it) {
         auto& tracker = *it;
-        if (tracker->usageCounter && tracker->metadata.rangeOverlapsChunk(range)) {
+        if (tracker->usageCounter && tracker->metadata &&
+            tracker->metadata->rangeOverlapsChunk(range)) {
             return tracker.get();
         }
     }
@@ -572,7 +605,7 @@ auto MetadataManager::_overlapsInUseCleanups(WithLock, ChunkRange const& range)
 boost::optional<ChunkRange> MetadataManager::getNextOrphanRange(BSONObj const& from) const {
     stdx::lock_guard<stdx::mutex> lg(_managerLock);
     invariant(!_metadata.empty());
-    return _metadata.back()->metadata.getNextOrphanRange(_receivingChunks, from);
+    return _metadata.back()->metadata->getNextOrphanRange(_receivingChunks, from);
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/s/metadata_manager.h b/src/mongo/db/s/metadata_manager.h
index b28bd62c6cc..b6cbf10cbc7 100644
--- a/src/mongo/db/s/metadata_manager.h
+++ b/src/mongo/db/s/metadata_manager.h
@@ -79,6 +79,13 @@ public:
     size_t numberOfMetadataSnapshots() const;
 
     /**
+     * Returns the number of metadata objects that have been set to boost::none in
+     * _retireExpiredMetadata(). The actual number may vary after it returns, so this is really only
+     * useful for unit tests.
+     */
+    int numberOfEmptyMetadataSnapshots() const;
+
+    /**
      * Uses the contents of the specified metadata as a way to purge any pending chunks.
      */
     void refreshActiveMetadata(std::unique_ptr<CollectionMetadata> newMetadata);
@@ -159,7 +166,7 @@ private:
             invariant(!usageCounter);
         }
 
-        CollectionMetadata metadata;
+        boost::optional<CollectionMetadata> metadata;
 
         std::list<Deletion> orphans;
 
diff --git a/src/mongo/db/s/metadata_manager_test.cpp b/src/mongo/db/s/metadata_manager_test.cpp
index 5c201ade003..47241fdee53 100644
--- a/src/mongo/db/s/metadata_manager_test.cpp
+++ b/src/mongo/db/s/metadata_manager_test.cpp
@@ -316,5 +316,85 @@ TEST_F(MetadataManagerTest, RangesToCleanMembership) {
     notifn.abandon();
 }
 
+TEST_F(MetadataManagerTest, ClearUnneededChunkManagerObjectsLastSnapshotInList) {
+    _manager->refreshActiveMetadata(makeEmptyMetadata());
+    ChunkRange cr1(BSON("key" << 0), BSON("key" << 10));
+    ChunkRange cr2(BSON("key" << 30), BSON("key" << 40));
+
+    auto scm1 = _manager->getActiveMetadata(_manager, boost::none);
+    {
+        _manager->refreshActiveMetadata(cloneMetadataPlusChunk(scm1, cr1));
+        ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 1UL);
+        ASSERT_EQ(_manager->numberOfRangesToClean(), 0UL);
+
+        auto scm2 = _manager->getActiveMetadata(_manager, boost::none);
+        ASSERT_EQ(scm2->getChunks().size(), 1UL);
+        _manager->refreshActiveMetadata(cloneMetadataPlusChunk(scm2, cr2));
+        ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 2UL);
+        ASSERT_EQ(_manager->numberOfEmptyMetadataSnapshots(), 0);
+    }
+
+    // The CollectionMetadata in scm2 should be set to boost::none because the object accessing it
+    // is now out of scope, but that in scm1 should remain
+    ASSERT_EQ(_manager->numberOfEmptyMetadataSnapshots(), 1);
+    ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 2UL);
+    ASSERT_EQ((_manager->getActiveMetadata(_manager, boost::none))->getChunks().size(), 2UL);
+}
+
+TEST_F(MetadataManagerTest, ClearUnneededChunkManagerObjectSnapshotInMiddleOfList) {
+    _manager->refreshActiveMetadata(makeEmptyMetadata());
+    ChunkRange cr1(BSON("key" << 0), BSON("key" << 10));
+    ChunkRange cr2(BSON("key" << 30), BSON("key" << 40));
+    ChunkRange cr3(BSON("key" << 50), BSON("key" << 80));
+    ChunkRange cr4(BSON("key" << 90), BSON("key" << 100));
+
+    auto scm = _manager->getActiveMetadata(_manager, boost::none);
+    _manager->refreshActiveMetadata(cloneMetadataPlusChunk(scm, cr1));
+    ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 1UL);
+    ASSERT_EQ(_manager->numberOfRangesToClean(), 0UL);
+
+    auto scm2 = _manager->getActiveMetadata(_manager, boost::none);
+    ASSERT_EQ(scm2->getChunks().size(), 1UL);
+    _manager->refreshActiveMetadata(cloneMetadataPlusChunk(scm2, cr2));
+
+    {
+        auto scm3 = _manager->getActiveMetadata(_manager, boost::none);
+        ASSERT_EQ(scm3->getChunks().size(), 2UL);
+        _manager->refreshActiveMetadata(cloneMetadataPlusChunk(scm3, cr3));
+        ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 3UL);
+        ASSERT_EQ(_manager->numberOfEmptyMetadataSnapshots(), 0);
+
+        /**
+         * The CollectionMetadata object created when creating scm2 above will be set to boost::none
+         * when we overrwrite scm2 below. The _metadata list will then look like:
+         * [
+         *      CollectionMetadataTracker{ metadata: xxx, orphans: [], usageCounter: 1},
+         *      CollectionMetadataTracker{ metadata: boost::none, orphans: [], usageCounter: 0},
+         *      CollectionMetadataTracker{ metadata: xxx, orphans: [], usageCounter: 1},
+         *      CollectionMetadataTracker{ metadata: xxx, orphans: [], usageCounter: 1}
+         * ]
+         */
+        scm2 = _manager->getActiveMetadata(_manager, boost::none);
+        ASSERT_EQ(scm2->getChunks().size(), 3UL);
+        _manager->refreshActiveMetadata(cloneMetadataPlusChunk(scm2, cr4));
+        ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 4UL);
+        ASSERT_EQ(_manager->numberOfEmptyMetadataSnapshots(), 1);
+    }
+
+
+    /** The CollectionMetadata in scm3 should be set to boost::none because the object accessing it
+     * is now out of scope. The _metadata list should look like:
+     * [
+     *      CollectionMetadataTracker{ metadata: xxx, orphans: [], usageCounter: 1},
+     *      CollectionMetadataTracker{ metadata: boost::none, orphans: [], usageCounter: 0},
+     *      CollectionMetadataTracker{ metadata: boost::none, orphans: [], usageCounter: 0},
+     *      CollectionMetadataTracker{ metadata: xxx, orphans: [], usageCounter: 1}
+     * ]
+     */
+
+    ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 4UL);
+    ASSERT_EQ(_manager->numberOfEmptyMetadataSnapshots(), 2);
+}
+
 }  // namespace
 }  // namespace mongo