sharder: only shrink to root if all ranges are involved

Tighten up conditions for shrinking shards to their root container so
that *all* shard ranges must be included in the single compactible
sequence. Previously shard ranges in states that were not eligible for
shrinking (such as CREATED) would not prevent other shards shrinking
to root.

Change-Id: I3da9b59f6744b10853d89b51ae15f05bdd51d02d

2 files changed, 46 insertions, 9 deletions

diff --git a/swift/container/sharder.py b/swift/container/sharder.py
index 2616cf9e4..0bd73bc8e 100644
--- a/swift/container/sharder.py
+++ b/swift/container/sharder.py
@@ -262,16 +262,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 785369964..ea65f2817 100644
--- a/test/unit/container/test_sharder.py
+++ b/test/unit/container/test_sharder.py
@@ -6359,11 +6359,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):
@@ -6413,6 +6425,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