From 1b183f221d258819c82ee58961afc765529314ac Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Fri, 14 May 2021 13:59:37 +0100
Subject: [PATCH] sharder: report perfectly overlapping shard ranges

Previously, shard ranges that overlapped perfectly (i.e. their bounds
were equal) were erroneously excluded from overlapping shard reports.
With this patch they will be reported in logs and cause shard audit
failures.

Also, the format of the audit log line is simplified to avoid
unnecessary duplication of information.

Change-Id: Ie15c9f40a132374c89337f0009fb5cf5a8e62c51
Closes-Bug: #1928459
Related-Bug: #1913332
---
 swift/container/sharder.py          | 21 ++++++++++++---------
 test/unit/container/test_sharder.py | 13 ++++++++-----
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/swift/container/sharder.py b/swift/container/sharder.py
index a2d811f376..5abfb9f8eb 100644
--- a/swift/container/sharder.py
+++ b/swift/container/sharder.py
@@ -108,12 +108,13 @@ def find_overlapping_ranges(shard_ranges):
         each other.
     """
     result = set()
-    for shard_range in shard_ranges:
-        overlapping = [sr for sr in shard_ranges
-                       if shard_range != sr and shard_range.overlaps(sr)]
+    for i, shard_range in enumerate(shard_ranges):
+        overlapping = [
+            sr for sr in shard_ranges[i + 1:]
+            if shard_range.name != sr.name and shard_range.overlaps(sr)]
         if overlapping:
             overlapping.append(shard_range)
-            overlapping.sort()
+            overlapping.sort(key=ShardRange.sort_key)
             result.add(tuple(overlapping))
 
     return result
@@ -994,12 +995,14 @@ class ContainerSharder(ContainerReplicator):
                 continue
             shard_ranges = broker.get_shard_ranges(states=state)
             overlaps = find_overlapping_ranges(shard_ranges)
-            for overlapping_ranges in overlaps:
+            if overlaps:
+                all_overlaps = ', '.join(
+                    [' '.join(['%s-%s' % (sr.lower, sr.upper)
+                               for sr in overlapping_ranges])
+                     for overlapping_ranges in sorted(list(overlaps))])
                 warnings.append(
-                    'overlapping ranges in state %s: %s' %
-                    (ShardRange.STATES[state],
-                     ' '.join(['%s-%s' % (sr.lower, sr.upper)
-                               for sr in overlapping_ranges])))
+                    'overlapping ranges in state %r: %s' %
+                    (ShardRange.STATES[state], all_overlaps))
 
         if warnings:
             self.logger.warning(
diff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py
index a3cc933d08..ae7c3e5aa8 100644
--- a/test/unit/container/test_sharder.py
+++ b/test/unit/container/test_sharder.py
@@ -117,10 +117,10 @@ class BaseTestSharder(unittest.TestCase):
         if not isinstance(state, (tuple, list)):
             state = [state] * len(bounds)
         state_iter = iter(state)
-        return [ShardRange('.shards_a/c_%s' % upper, timestamp,
+        return [ShardRange('.shards_a/c_%s_%s' % (upper, index), timestamp,
                            lower, upper, state=next(state_iter),
                            object_count=object_count, **kwargs)
-                for lower, upper in bounds]
+                for index, (lower, upper) in enumerate(bounds)]
 
     def ts_encoded(self):
         # make a unique timestamp string with multiple timestamps encoded;
@@ -4859,11 +4859,14 @@ class TestSharder(BaseTestSharder):
             self.assertIn(
                 'Audit failed for root %s' % broker.db_file, line)
             self.assertIn(
-                'overlapping ranges in state %s: k-t s-z' % state_text,
-                line)
+                'overlapping ranges in state %r: k-t s-y, y-z y-z'
+                % state_text, line)
+            # check for no duplicates in reversed order
+            self.assertNotIn('s-z k-t', line)
 
         expected_stats = {'attempted': 1, 'success': 0, 'failure': 1}
-        shard_bounds = (('a', 'j'), ('k', 't'), ('s', 'z'))
+        shard_bounds = (('a', 'j'), ('k', 't'), ('s', 'y'),
+                        ('y', 'z'), ('y', 'z'))
         for state, state_text in ShardRange.STATES.items():
             if state in (ShardRange.SHRINKING,
                          ShardRange.SHARDED,