sharder: don't cleave DB if own_shard_range missing

The sharder aborts cleaving shards if the root DB does not have an
own_shard_range (see Related-Change). Previously, the cleaving was
aborted *after* a shard broker had been created, which potentially
leaves an unused shard DB in a handoff location. This shard DB would
eventually be replicated and cleaned up by the replicator, but its
creation can be avoided by checking the root DB own_shard_range
earlier.

Change-Id: I05ec542faa452db2053302910d34b93f0abd10a7
Related-Change: I68c3e7558d04e8dcd5874f80b4763ec0b820331d

2 files changed, 36 insertions, 25 deletions

diff --git a/swift/container/sharder.py b/swift/container/sharder.py
index db6565fd5..fb5b51fcf 100644
--- a/swift/container/sharder.py
+++ b/swift/container/sharder.py
@@ -1603,7 +1603,8 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
                 len(created_ranges))
         return len(created_ranges)
 
-    def _cleave_shard_range(self, broker, cleaving_context, shard_range):
+    def _cleave_shard_range(self, broker, cleaving_context, shard_range,
+                            own_shard_range):
         self.logger.info("Cleaving '%s' from row %s into %s for %r",
                          quote(broker.path),
                          cleaving_context.last_cleave_to_row,
@@ -1620,15 +1621,6 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
             self._increment_stat('cleaved', 'failure', statsd=True)
             return CLEAVE_FAILED
 
-        own_shard_range = broker.get_own_shard_range(no_default=True)
-        if own_shard_range is None:
-            # A default should never be SHRINKING or SHRUNK but because we
-            # may write own_shard_range back to broker, let's make sure
-            # it can't be defaulted.
-            self.logger.warning('Failed to get own_shard_range for %s',
-                                quote(broker.path))
-            return CLEAVE_FAILED
-
         # only cleave from the retiring db - misplaced objects handler will
         # deal with any objects in the fresh db
         source_broker = broker.get_brokers()[0]
@@ -1796,6 +1788,15 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
             self.logger.debug('Starting to cleave (%s todo): %s',
                               cleaving_context.ranges_todo, quote(broker.path))
 
+        own_shard_range = broker.get_own_shard_range(no_default=True)
+        if own_shard_range is None:
+            # A default should never be SHRINKING or SHRUNK but because we
+            # may write own_shard_range back to broker, let's make sure
+            # it can't be defaulted.
+            self.logger.warning('Failed to get own_shard_range for %s',
+                                quote(broker.path))
+            ranges_todo = []  # skip cleaving
+
         ranges_done = []
         for shard_range in ranges_todo:
             if cleaving_context.cleaving_done:
@@ -1820,7 +1821,7 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
                 break
 
             cleave_result = self._cleave_shard_range(
-                broker, cleaving_context, shard_range)
+                broker, cleaving_context, shard_range, own_shard_range)
 
             if cleave_result == CLEAVE_SUCCESS:
                 ranges_done.append(shard_range)
diff --git a/test/unit/container/test_sharder.py b/test/unit/container/test_sharder.py
index 75a8816aa..ef310268f 100644
--- a/test/unit/container/test_sharder.py
+++ b/test/unit/container/test_sharder.py
@@ -42,8 +42,7 @@ from swift.container.sharder import ContainerSharder, sharding_enabled, \
     CleavingContext, DEFAULT_SHARDER_CONF, finalize_shrinking, \
     find_shrinking_candidates, process_compactible_shard_sequences, \
     find_compactible_shard_sequences, is_shrinking_candidate, \
-    is_sharding_candidate, find_paths, rank_paths, ContainerSharderConf, \
-    CLEAVE_FAILED
+    is_sharding_candidate, find_paths, rank_paths, ContainerSharderConf
 from swift.common.utils import ShardRange, Timestamp, hash_path, \
     encode_timestamps, parse_db_filename, quorum_size, Everything, md5
 from test import annotate_failure
@@ -1696,25 +1695,36 @@ class TestSharder(BaseTestSharder):
         self.assertFalse(cleaving_context.cleaving_done)
 
     def test_cleave_shard_range_no_own_shard_range(self):
-        broker = self._make_sharding_broker()
+        # create an unsharded broker that has shard ranges but no
+        # own_shard_range, verify that it does not cleave...
+        broker = self._make_broker()
+        broker.set_sharding_sysmeta('Quoted-Root', 'a/c')
+        shard_ranges = self._make_shard_ranges(
+            (('', 'middle'), ('middle', '')),
+            state=ShardRange.CLEAVED)
+        broker.merge_shard_ranges(shard_ranges)
         obj = {'name': 'obj', 'created_at': next(self.ts_iter).internal,
                'size': 14, 'content_type': 'text/plain', 'etag': 'an etag',
                'deleted': 0}
         broker.get_brokers()[0].merge_items([obj])
-        self.assertEqual(2, len(broker.db_files))  # sanity check
-        context = CleavingContext.load(broker)
-        shard_range = broker.get_shard_ranges()[0]
-
-        with self._mock_sharder() as sharder, mock.patch(
-                'swift.container.backend.ContainerBroker.get_own_shard_range',
-                return_value=None):
-            self.assertEqual(
-                sharder._cleave_shard_range(broker, context, shard_range),
-                CLEAVE_FAILED)
-        self.assertEqual(SHARDING, broker.get_db_state())
+        with self._mock_sharder() as sharder:
+            self.assertFalse(sharder._cleave(broker))
+        self.assertEqual(UNSHARDED, broker.get_db_state())
         warning_lines = sharder.logger.get_lines_for_level('warning')
         self.assertEqual(warning_lines[0],
                          'Failed to get own_shard_range for a/c')
+        sharder._replicate_object.assert_not_called()
+        context = CleavingContext.load(broker)
+        self.assertTrue(context.misplaced_done)
+        self.assertFalse(context.cleaving_done)
+        # only the root broker on disk
+        suffix_dir = os.path.dirname(broker.db_dir)
+        self.assertEqual([os.path.basename(broker.db_dir)],
+                         os.listdir(suffix_dir))
+        partition_dir = os.path.dirname(suffix_dir)
+        self.assertEqual([broker.db_dir[-3:]], os.listdir(partition_dir))
+        containers_dir = os.path.dirname(partition_dir)
+        self.assertEqual(['0'], os.listdir(containers_dir))
 
     def test_cleave_shard(self):
         broker = self._make_broker(account='.shards_a', container='shard_c')