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

3 files changed, 136 insertions, 18 deletions

diff --git a/src/mongo/db/s/metadata_manager.cpp b/src/mongo/db/s/metadata_manager.cpp
index 94679545535..b111875db39 100644
--- a/src/mongo/db/s/metadata_manager.cpp
+++ b/src/mongo/db/s/metadata_manager.cpp
@@ -201,8 +201,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:
@@ -253,12 +256,12 @@ boost::optional<ScopedCollectionMetadata> MetadataManager::getActiveMetadata(
 
     // We don't keep routing history for unsharded collections, so if the collection is unsharded
     // just return the active metadata
-    if (!atClusterTime || !activeMetadata.isSharded()) {
+    if (!atClusterTime || !activeMetadata->isSharded()) {
         return ScopedCollectionMetadata(std::make_shared<RangePreserver>(
             lg, std::move(self), std::move(activeMetadataTracker)));
     }
 
-    auto chunkManager = activeMetadata.getChunkManager();
+    auto chunkManager = activeMetadata->getChunkManager();
     auto chunkManagerAtClusterTime = std::make_shared<ChunkManager>(
         chunkManager->getRoutingHistory(), atClusterTime->asTimestamp());
 
@@ -275,7 +278,7 @@ boost::optional<ScopedCollectionMetadata> MetadataManager::getActiveMetadata(
     };
 
     return ScopedCollectionMetadata(std::make_shared<MetadataAtTimestamp>(
-        CollectionMetadata(chunkManagerAtClusterTime, activeMetadata.shardId())));
+        CollectionMetadata(chunkManagerAtClusterTime, activeMetadata->shardId())));
 }
 
 size_t MetadataManager::numberOfMetadataSnapshots() const {
@@ -286,6 +289,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::setFilteringMetadata(CollectionMetadata remoteMetadata) {
     stdx::lock_guard<stdx::mutex> lg(_managerLock);
 
@@ -304,9 +319,9 @@ void MetadataManager::setFilteringMetadata(CollectionMetadata remoteMetadata) {
 
     // 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
-    if (activeMetadata.getCollVersion().epoch() != remoteMetadata.getCollVersion().epoch()) {
+    if (activeMetadata->getCollVersion().epoch() != remoteMetadata.getCollVersion().epoch()) {
         LOG(0) << "Updating metadata for collection " << _nss.ns() << " from "
-               << activeMetadata.toStringBasic() << " to " << remoteMetadata.toStringBasic()
+               << activeMetadata->toStringBasic() << " to " << remoteMetadata.toStringBasic()
                << " due to epoch change";
 
         _receivingChunks.clear();
@@ -318,14 +333,14 @@ void MetadataManager::setFilteringMetadata(CollectionMetadata remoteMetadata) {
     }
 
     // We already have the same or newer version
-    if (activeMetadata.getCollVersion() >= remoteMetadata.getCollVersion()) {
-        LOG(1) << "Ignoring update of active metadata " << activeMetadata.toStringBasic()
+    if (activeMetadata->getCollVersion() >= remoteMetadata.getCollVersion()) {
+        LOG(1) << "Ignoring update of active metadata " << activeMetadata->toStringBasic()
                << " with an older " << remoteMetadata.toStringBasic();
         return;
     }
 
     LOG(0) << "Updating metadata for collection " << _nss.ns() << " from "
-           << activeMetadata.toStringBasic() << " to " << remoteMetadata.toStringBasic()
+           << activeMetadata->toStringBasic() << " to " << remoteMetadata.toStringBasic()
            << " due to version change";
 
     // Resolve any receiving chunks, which might have completed by now
@@ -362,18 +377,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(0) << "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 {
@@ -406,7 +436,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);
@@ -428,7 +458,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);
     }
 }
 
@@ -535,14 +565,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();
         }
     }
@@ -575,7 +606,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 4112d0fe4d8..26cb452ac28 100644
--- a/src/mongo/db/s/metadata_manager.h
+++ b/src/mongo/db/s/metadata_manager.h
@@ -81,6 +81,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;
+
     void setFilteringMetadata(CollectionMetadata newMetadata);
 
     void clearFilteringMetadata();
@@ -162,7 +169,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 ccce39d0521..db701ec0a60 100644
--- a/src/mongo/db/s/metadata_manager_test.cpp
+++ b/src/mongo/db/s/metadata_manager_test.cpp
@@ -402,5 +402,85 @@ TEST_F(MetadataManagerTest, RangesToCleanMembership) {
     notifn.abandon();
 }
 
+TEST_F(MetadataManagerTest, ClearUnneededChunkManagerObjectsLastSnapshotInList) {
+    _manager->setFilteringMetadata(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->setFilteringMetadata(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->setFilteringMetadata(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->setFilteringMetadata(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->setFilteringMetadata(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->setFilteringMetadata(cloneMetadataPlusChunk(scm2, cr2));
+
+    {
+        auto scm3 = *_manager->getActiveMetadata(_manager, boost::none);
+        ASSERT_EQ(scm3->getChunks().size(), 2UL);
+        _manager->setFilteringMetadata(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->setFilteringMetadata(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