Merge "sharder: only shrink to root if all ranges are involved"

2 files changed, 46 insertions, 9 deletions

diff --git a/swift/container/sharder.py b/swift/container/sharder.py
index 9a7b7e90d..12ee4c0f5 100644
--- a/swift/container/sharder.py
+++ b/swift/container/sharder.py
@@ -261,16 +261,16 @@ def find_compactible_shard_sequences(broker,
 
         index += len(sequence)
         if (index == len(shard_ranges) and
-                not compactible_sequences and
+                len(shard_ranges) == len(sequence) and
                 not sequence_complete(sequence) and
                 sequence.includes(own_shard_range)):
-            # special case: only one sequence has been found, which encompasses
-            # the entire namespace, has no more than merge_size records and
-            # whose shard ranges are all shrinkable; all the shards in the
-            # sequence can be shrunk to the root, so append own_shard_range to
-            # the sequence to act as an acceptor; note: only shrink to the root
-            # when *all* the remaining shard ranges can be simultaneously
-            # shrunk to the root.
+            # special case: only one sequence has been found, which consumes
+            # all shard ranges, encompasses the entire namespace, has no more
+            # than merge_size records and whose shard ranges are all
+            # shrinkable; all the shards in the sequence can be shrunk to the
+            # root, so append own_shard_range to the sequence to act as an
+            # acceptor; note: only shrink to the root when *all* the remaining
+            # shard ranges can be simultaneously shrunk to the root.
             sequence.append(own_shard_range)
 
         if len(sequence) < 2 or sequence[-1].state not in (ShardRange.ACTIVE,
diff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py
index a8c65c23e..28d0e6840 100644
--- a/test/unit/container/test_sharder.py
+++ b/test/unit/container/test_sharder.py
@@ -6517,11 +6517,23 @@ class TestSharderFunctions(BaseTestSharder):
         sequences = find_compactible_shard_sequences(broker, 20, 19, -1, -1)
         self.assertEqual([], sequences)
 
+    def test_find_compactible_nine_donors_one_acceptor(self):
+        # one sequence that spans entire namespace but does not shrink to root
+        broker = self._make_broker()
+        shard_ranges = self._make_shard_ranges(
+            (('', 'b'), ('b', 'c'), ('c', 'd'), ('d', 'e'), ('e', 'f'),
+             ('f', 'g'), ('g', 'h'), ('h', 'i'), ('i', 'j'), ('j', '')),
+            state=ShardRange.ACTIVE)
+        shard_ranges[9].object_count = 11  # final shard too big to shrink
+        broker.merge_shard_ranges(shard_ranges)
+        sequences = find_compactible_shard_sequences(broker, 10, 999, -1, -1)
+        self.assertEqual([shard_ranges], sequences)
+
     def test_find_compactible_four_donors_two_acceptors(self):
         small_ranges = (2, 3, 4, 7)
         broker = self._make_broker()
         shard_ranges = self._make_shard_ranges(
-            (('a', 'b'), ('b', 'c'), ('c', 'd'), ('d', 'e'), ('e', 'f'),
+            (('', 'b'), ('b', 'c'), ('c', 'd'), ('d', 'e'), ('e', 'f'),
              ('f', 'g'), ('g', 'h'), ('h', 'i'), ('i', 'j'), ('j', '')),
             state=ShardRange.ACTIVE)
         for i, sr in enumerate(shard_ranges):
@@ -6571,6 +6583,31 @@ class TestSharderFunctions(BaseTestSharder):
                                                      include_shrinking=True)
         self.assertEqual([shard_ranges + [own_sr]], sequences)
 
+    def test_find_compactible_overlapping_ranges(self):
+        # unexpected case: all shrinkable, two overlapping sequences, one which
+        # spans entire namespace; should not shrink to root
+        broker = self._make_broker()
+        shard_ranges = self._make_shard_ranges(
+            (('', 'b'), ('b', 'c'),  # overlaps form one sequence
+             ('', 'j'), ('j', '')),  # second sequence spans entire namespace
+            state=ShardRange.ACTIVE)
+        shard_ranges[1].object_count = 11  # cannot shrink, so becomes acceptor
+        broker.merge_shard_ranges(shard_ranges)
+        sequences = find_compactible_shard_sequences(broker, 10, 999, -1, -1)
+        self.assertEqual([shard_ranges[:2], shard_ranges[2:]], sequences)
+
+    def test_find_compactible_overlapping_ranges_with_ineligible_state(self):
+        # unexpected case: one ineligible state shard range overlapping one
+        # sequence which spans entire namespace; should not shrink to root
+        broker = self._make_broker()
+        shard_ranges = self._make_shard_ranges(
+            (('', 'b'),  # overlap in ineligible state
+             ('', 'j'), ('j', '')),  # sequence spans entire namespace
+            state=[ShardRange.CREATED, ShardRange.ACTIVE, ShardRange.ACTIVE])
+        broker.merge_shard_ranges(shard_ranges)
+        sequences = find_compactible_shard_sequences(broker, 10, 999, -1, -1)
+        self.assertEqual([shard_ranges[1:]], sequences)
+
     def test_find_compactible_donors_but_no_suitable_acceptor(self):
         # if shard ranges are already shrinking, check that the final one is
         # not made into an acceptor if a suitable adjacent acceptor is not