SERVER-30058 Balancer policy should not move chunks off shards on 'size exceeded' conditions

(cherry picked from commit 411114cb3ec3119bb159b29b8ef65292e4d20de3)

4 files changed, 23 insertions, 25 deletions

diff --git a/src/mongo/db/s/balancer/balancer_policy.cpp b/src/mongo/db/s/balancer/balancer_policy.cpp
index 206a50257a6..348d8cec603 100644
--- a/src/mongo/db/s/balancer/balancer_policy.cpp
+++ b/src/mongo/db/s/balancer/balancer_policy.cpp
@@ -222,8 +222,7 @@ Status BalancerPolicy::isShardSuitableReceiver(const ClusterStatistics::ShardSta
                                                const string& chunkTag) {
     if (stat.isSizeMaxed()) {
         return {ErrorCodes::IllegalOperation,
-                str::stream() << stat.shardId
-                              << " has already reached the maximum total chunk size."};
+                str::stream() << stat.shardId << " has reached its maximum storage size."};
     }
 
     if (stat.isDraining) {
@@ -299,11 +298,10 @@ vector<MigrateInfo> BalancerPolicy::balance(const ShardStatisticsVector& shardSt
     // migrations for the same shard.
     set<ShardId> usedShards;
 
-    // 1) Check for shards, which are in draining mode or are above the size limit and must have
-    // chunks moved off of them
+    // 1) Check for shards, which are in draining mode
     {
         for (const auto& stat : shardStats) {
-            if (!stat.isDraining && !stat.isSizeExceeded())
+            if (!stat.isDraining)
                 continue;
 
             if (usedShards.count(stat.shardId))
diff --git a/src/mongo/db/s/balancer/balancer_policy_test.cpp b/src/mongo/db/s/balancer/balancer_policy_test.cpp
index 11fe57e133d..de5ceafaa83 100644
--- a/src/mongo/db/s/balancer/balancer_policy_test.cpp
+++ b/src/mongo/db/s/balancer/balancer_policy_test.cpp
@@ -408,22 +408,36 @@ TEST(BalancerPolicy, NoBalancingDueToAllNodesEitherDrainingOrMaxedOut) {
     ASSERT(migrations.empty());
 }
 
-TEST(BalancerPolicy, BalancerMovesChunksOffSizeMaxedShards) {
+TEST(BalancerPolicy, BalancerRespectsMaxShardSizeOnlyBalanceToNonMaxed) {
     // Note that maxSize of shard0 is 1, and it is therefore overloaded with currSize = 3. Other
     // shards have maxSize = 0 = unset. Even though the overloaded shard has the least number of
     // less chunks, we shouldn't move chunks to that shard.
     auto cluster = generateCluster(
         {{ShardStatistics(kShardId0, 1, 3, false, emptyTagSet, emptyShardVersion), 2},
-         {ShardStatistics(kShardId1, kNoMaxSize, 4, false, emptyTagSet, emptyShardVersion), 4},
-         {ShardStatistics(kShardId2, kNoMaxSize, 6, false, emptyTagSet, emptyShardVersion), 6}});
+         {ShardStatistics(kShardId1, kNoMaxSize, 5, false, emptyTagSet, emptyShardVersion), 5},
+         {ShardStatistics(kShardId2, kNoMaxSize, 10, false, emptyTagSet, emptyShardVersion), 10}});
 
     const auto migrations(BalancerPolicy::balance(
         cluster.first, DistributionStatus(kNamespace, cluster.second), false));
     ASSERT_EQ(1U, migrations.size());
-    ASSERT_EQ(kShardId0, migrations[0].from);
+    ASSERT_EQ(kShardId2, migrations[0].from);
     ASSERT_EQ(kShardId1, migrations[0].to);
-    ASSERT_BSONOBJ_EQ(cluster.second[kShardId0][0].getMin(), migrations[0].minKey);
-    ASSERT_BSONOBJ_EQ(cluster.second[kShardId0][0].getMax(), migrations[0].maxKey);
+    ASSERT_BSONOBJ_EQ(cluster.second[kShardId2][0].getMin(), migrations[0].minKey);
+    ASSERT_BSONOBJ_EQ(cluster.second[kShardId2][0].getMax(), migrations[0].maxKey);
+}
+
+TEST(BalancerPolicy, BalancerRespectsMaxShardSizeWhenAllBalanced) {
+    // Note that maxSize of shard0 is 1, and it is therefore overloaded with currSize = 4. Other
+    // shards have maxSize = 0 = unset. We check that being over the maxSize is NOT equivalent to
+    // draining, we don't want to empty shards for no other reason than they are over this limit.
+    auto cluster = generateCluster(
+        {{ShardStatistics(kShardId0, 1, 4, false, emptyTagSet, emptyShardVersion), 4},
+         {ShardStatistics(kShardId1, kNoMaxSize, 4, false, emptyTagSet, emptyShardVersion), 4},
+         {ShardStatistics(kShardId2, kNoMaxSize, 4, false, emptyTagSet, emptyShardVersion), 4}});
+
+    const auto migrations(BalancerPolicy::balance(
+        cluster.first, DistributionStatus(kNamespace, cluster.second), false));
+    ASSERT(migrations.empty());
 }
 
 TEST(BalancerPolicy, BalancerRespectsTagsWhenDraining) {
diff --git a/src/mongo/db/s/balancer/cluster_statistics.cpp b/src/mongo/db/s/balancer/cluster_statistics.cpp
index 125f11d3551..d51b4ec4a24 100644
--- a/src/mongo/db/s/balancer/cluster_statistics.cpp
+++ b/src/mongo/db/s/balancer/cluster_statistics.cpp
@@ -60,14 +60,6 @@ bool ClusterStatistics::ShardStatistics::isSizeMaxed() const {
     return currSizeMB >= maxSizeMB;
 }
 
-bool ClusterStatistics::ShardStatistics::isSizeExceeded() const {
-    if (!maxSizeMB || !currSizeMB) {
-        return false;
-    }
-
-    return currSizeMB > maxSizeMB;
-}
-
 BSONObj ClusterStatistics::ShardStatistics::toBSON() const {
     BSONObjBuilder builder;
     builder.append("id", shardId.toString());
diff --git a/src/mongo/db/s/balancer/cluster_statistics.h b/src/mongo/db/s/balancer/cluster_statistics.h
index 2717c42b7ee..a8e433062e8 100644
--- a/src/mongo/db/s/balancer/cluster_statistics.h
+++ b/src/mongo/db/s/balancer/cluster_statistics.h
@@ -71,12 +71,6 @@ public:
         bool isSizeMaxed() const;
 
         /**
-         * Returns true if a shard must be relieved (if possible) of some of the chunks it hosts
-         * because it has exceeded its per-shard data size limit.
-         */
-        bool isSizeExceeded() const;
-
-        /**
          * Returns BSON representation of this shard's statistics, for reporting purposes.
          */
         BSONObj toBSON() const;