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

(cherry picked from commit da8c0d18e7ba69ef2ce31236d34816f6fbe8cec3)

3 files changed, 141 insertions, 17 deletions

diff --git a/src/mongo/db/s/metadata_manager.cpp b/src/mongo/db/s/metadata_manager.cpp
index bcfab6364e6..87e59ee84e9 100644
--- a/src/mongo/db/s/metadata_manager.cpp
+++ b/src/mongo/db/s/metadata_manager.cpp
@@ -198,6 +198,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);
 
@@ -213,7 +225,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);
@@ -232,7 +244,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
@@ -284,17 +296,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.
+            LOG(0) << "Queries possibly dependent on " << _nss.ns()
+                   << " range(s) finished; scheduling ranges for deletion";
+
             _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 {
@@ -327,7 +355,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);
@@ -349,7 +377,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());
 }
@@ -436,7 +464,7 @@ std::vector<ScopedCollectionMetadata> MetadataManager::overlappingMetadata(
 
     // Start with the active metadata
     auto it = _metadata.rbegin();
-    if ((*it)->metadata.rangeOverlapsChunk(range)) {
+    if ((*it)->metadata->rangeOverlapsChunk(range)) {
         // We ignore the refcount of the active mapping; effectively, we assume it is in use.
         result.push_back(ScopedCollectionMetadata(lg, self, (*it)));
     }
@@ -447,7 +475,7 @@ std::vector<ScopedCollectionMetadata> MetadataManager::overlappingMetadata(
         auto& tracker = *it;
 
         // We want all the overlapping snapshot mappings still possibly in use by a query.
-        if (tracker->usageCounter > 0 && tracker->metadata.rangeOverlapsChunk(range)) {
+        if (tracker->usageCounter > 0 && tracker->metadata->rangeOverlapsChunk(range)) {
             result.push_back(ScopedCollectionMetadata(lg, self, tracker));
         }
     }
@@ -485,14 +513,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();
         }
     }
@@ -525,7 +554,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);
 }
 
 ScopedCollectionMetadata::ScopedCollectionMetadata() = default;
@@ -558,13 +587,14 @@ ScopedCollectionMetadata& ScopedCollectionMetadata::operator=(ScopedCollectionMe
 }
 
 CollectionMetadata* ScopedCollectionMetadata::getMetadata() const {
-    return _metadataTracker ? &_metadataTracker->metadata : nullptr;
+    return _metadataTracker && _metadataTracker->metadata ? &_metadataTracker->metadata.get()
+                                                          : nullptr;
 }
 
 BSONObj ScopedCollectionMetadata::extractDocumentKey(BSONObj const& doc) const {
     BSONObj key;
     if (*this) {  // is sharded
-        auto const& pattern = _metadataTracker->metadata.getChunkManager()->getShardKeyPattern();
+        auto const& pattern = _metadataTracker->metadata->getChunkManager()->getShardKeyPattern();
         key = dotted_path_support::extractElementsBasedOnTemplate(doc, pattern.toBSON());
         if (pattern.hasId()) {
             return key;
diff --git a/src/mongo/db/s/metadata_manager.h b/src/mongo/db/s/metadata_manager.h
index cbac10ab3d4..5a6cbda9d38 100644
--- a/src/mongo/db/s/metadata_manager.h
+++ b/src/mongo/db/s/metadata_manager.h
@@ -78,6 +78,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);
@@ -165,7 +172,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 0dcfe518973..3f8ce528555 100644
--- a/src/mongo/db/s/metadata_manager_test.cpp
+++ b/src/mongo/db/s/metadata_manager_test.cpp
@@ -27,7 +27,7 @@
  *    exception statement from all source files in the program, then also delete
  *    it in the license file.
  */
-
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kSharding
 #include "mongo/platform/basic.h"
 
 #include <boost/optional.hpp>
@@ -54,6 +54,7 @@
 #include "mongo/stdx/memory.h"
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/assert_util.h"
+#include "mongo/util/log.h"
 
 namespace mongo {
 namespace {
@@ -321,5 +322,91 @@ 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);
+    {
+        _manager->refreshActiveMetadata(
+            cloneMetadataPlusChunk(*scm1.getMetadata(), cr1.getMin(), cr1.getMax()));
+        ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 1UL);
+        ASSERT_EQ(_manager->numberOfRangesToClean(), 0UL);
+
+        auto scm2 = _manager->getActiveMetadata(_manager);
+        ASSERT_EQ(scm2->getChunks().size(), 1UL);
+        _manager->refreshActiveMetadata(
+            cloneMetadataPlusChunk(*scm2.getMetadata(), cr2.getMin(), cr2.getMax()));
+        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))->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);
+    _manager->refreshActiveMetadata(
+        cloneMetadataPlusChunk(*scm.getMetadata(), cr1.getMin(), cr1.getMax()));
+    ASSERT_EQ(_manager->numberOfMetadataSnapshots(), 1UL);
+    ASSERT_EQ(_manager->numberOfRangesToClean(), 0UL);
+
+    auto scm2 = _manager->getActiveMetadata(_manager);
+    ASSERT_EQ(scm2->getChunks().size(), 1UL);
+    _manager->refreshActiveMetadata(
+        cloneMetadataPlusChunk(*scm2.getMetadata(), cr2.getMin(), cr2.getMax()));
+
+    {
+        auto scm3 = _manager->getActiveMetadata(_manager);
+        ASSERT_EQ(scm3->getChunks().size(), 2UL);
+        _manager->refreshActiveMetadata(
+            cloneMetadataPlusChunk(*scm3.getMetadata(), cr3.getMin(), cr3.getMax()));
+        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);
+        ASSERT_EQ(scm2->getChunks().size(), 3UL);
+        _manager->refreshActiveMetadata(
+            cloneMetadataPlusChunk(*scm2.getMetadata(), cr4.getMin(), cr4.getMax()));
+        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