Merge "Remove per-device reconstruction stats"

swift/obj/reconstructor.py

@@ -366,32 +366,22 @@ class ObjectReconstructor(Daemon):
         """
         Logs various stats for the currently running reconstruction pass.
         """
-        if (self.device_count and self.part_count and
-                self.reconstruction_device_count):
+        if (self.device_count and self.part_count):
             elapsed = (time.time() - self.start) or 0.000001
             rate = self.reconstruction_part_count / elapsed
-            total_part_count = (1.0 * self.part_count *
-                                self.device_count /
-                                self.reconstruction_device_count)
             self.logger.info(
                 _("%(reconstructed)d/%(total)d (%(percentage).2f%%)"
-                  " partitions of %(device)d/%(dtotal)d "
-                  "(%(dpercentage).2f%%) devices"
-                  " reconstructed in %(time).2fs "
+                  " partitions reconstructed in %(time).2fs "
                   "(%(rate).2f/sec, %(remaining)s remaining)"),
                 {'reconstructed': self.reconstruction_part_count,
                  'total': self.part_count,
                  'percentage':
                  self.reconstruction_part_count * 100.0 / self.part_count,
-                 'device': self.reconstruction_device_count,
-                 'dtotal': self.device_count,
-                 'dpercentage':
-                 self.reconstruction_device_count * 100.0 / self.device_count,
                  'time': time.time() - self.start, 'rate': rate,
                  'remaining': '%d%s' %
                  compute_eta(self.start,
                              self.reconstruction_part_count,
-                             total_part_count)})
+                             self.part_count)})
 
             if self.suffix_count and self.partition_times:
                 self.logger.info(
@@ -842,7 +832,6 @@ class ObjectReconstructor(Daemon):
         for policy, local_devices in policy2devices.items():
             df_mgr = self._df_router[policy]
             for local_dev in local_devices:
-                self.reconstruction_device_count += 1
                 dev_path = df_mgr.get_dev_path(local_dev['device'])
                 if not dev_path:
                     self.logger.warning(_('%s is not mounted'),
@@ -876,7 +865,7 @@ class ObjectReconstructor(Daemon):
                     if not partition.isdigit():
                         self.logger.warning(
                             'Unexpected entity in data dir: %r' % part_path)
-                        remove_file(part_path)
+                        self.delete_partition(part_path)
                         self.reconstruction_part_count += 1
                         continue
                     partition = int(partition)
@@ -916,14 +905,16 @@ class ObjectReconstructor(Daemon):
         self.suffix_hash = 0
         self.reconstruction_count = 0
         self.reconstruction_part_count = 0
-        self.reconstruction_device_count = 0
         self.last_reconstruction_count = -1
         self.handoffs_remaining = 0
 
     def delete_partition(self, path):
+        def kill_it(path):
+            shutil.rmtree(path, ignore_errors=True)
+            remove_file(path)
+
         self.logger.info(_("Removing partition: %s"), path)
-        tpool.execute(shutil.rmtree, path, ignore_errors=True)
-        remove_file(path)
+        tpool.execute(kill_it, path)
 
     def reconstruct(self, **kwargs):
         """Run a reconstruction pass"""

test/unit/obj/test_reconstructor.py

@@ -620,9 +620,9 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         self._run_once(18, extra_devices)
         stats_lines = set()
         for line in self.logger.get_lines_for_level('info'):
-            if 'devices reconstructed in' not in line:
+            if 'reconstructed in' not in line:
                 continue
-            stat_line = line.split('of', 1)[0].strip()
+            stat_line = line.split('reconstructed', 1)[0].strip()
             stats_lines.add(stat_line)
         acceptable = set([
             '3/8 (37.50%) partitions',
@@ -633,7 +633,6 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         self.assertEqual(matched, acceptable,
                          'missing some expected acceptable:\n%s' % (
                              '\n'.join(sorted(acceptable - matched))))
-        self.assertEqual(self.reconstructor.reconstruction_device_count, 4)
         self.assertEqual(self.reconstructor.reconstruction_part_count, 8)
         self.assertEqual(self.reconstructor.part_count, 8)
 
@@ -647,9 +646,9 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         self._run_once(2, extra_devices, 'sdc1')
         stats_lines = set()
         for line in self.logger.get_lines_for_level('info'):
-            if 'devices reconstructed in' not in line:
+            if 'reconstructed in' not in line:
                 continue
-            stat_line = line.split('of', 1)[0].strip()
+            stat_line = line.split('reconstructed', 1)[0].strip()
             stats_lines.add(stat_line)
         acceptable = set([
             '1/1 (100.00%) partitions',
@@ -658,7 +657,6 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
         self.assertEqual(matched, acceptable,
                          'missing some expected acceptable:\n%s' % (
                              '\n'.join(sorted(acceptable - matched))))
-        self.assertEqual(self.reconstructor.reconstruction_device_count, 1)
         self.assertEqual(self.reconstructor.reconstruction_part_count, 1)
         self.assertEqual(self.reconstructor.part_count, 1)
 
@@ -1567,25 +1565,34 @@ class TestObjectReconstructor(unittest.TestCase):
         self.assertTrue('Unable to list partitions' in line)
         self.assertTrue(datadir_path in line)
 
-    def test_collect_parts_removes_non_partition_files(self):
+    def test_reconstruct_removes_non_partition_files(self):
         # create some junk next to partitions
         datadir_path = os.path.join(self.devices, self.local_dev['device'],
                                     diskfile.get_data_dir(self.policy))
         num_parts = 3
         for part in range(num_parts):
             utils.mkdirs(os.path.join(datadir_path, str(part)))
-        junk_file = os.path.join(datadir_path, 'junk')
-        with open(junk_file, 'w') as f:
-            f.write('junk')
+
+        # Add some clearly non-partition dentries
+        utils.mkdirs(os.path.join(datadir_path, 'not/a/partition'))
+        for junk_name in ('junk', '1234'):
+            junk_file = os.path.join(datadir_path, junk_name)
+            with open(junk_file, 'w') as f:
+                f.write('junk')
+
         with mock.patch('swift.obj.reconstructor.whataremyips',
-                        return_value=[self.ip]):
-            part_infos = list(self.reconstructor.collect_parts())
-        # the file is not included in the part_infos map
-        self.assertEqual(sorted(p['part_path'] for p in part_infos),
-                         sorted([os.path.join(datadir_path, str(i))
-                                 for i in range(num_parts)]))
-        # and gets cleaned up
-        self.assertFalse(os.path.exists(junk_file))
+                        return_value=[self.ip]), \
+                mock.patch('swift.obj.reconstructor.'
+                           'ObjectReconstructor.process_job'):
+            self.reconstructor.reconstruct()
+
+        # all the bad gets cleaned up
+        errors = []
+        for junk_name in ('junk', '1234', 'not'):
+            junk_file = os.path.join(datadir_path, junk_name)
+            if os.path.exists(junk_file):
+                errors.append('%s still exists!' % junk_file)
+        self.assertFalse(errors)
 
     def test_collect_parts_overrides(self):
         # setup multiple devices, with multiple parts