diff --git a/swift/obj/reconstructor.py b/swift/obj/reconstructor.py index b0f35cb6d1..28e148b940 100644 --- a/swift/obj/reconstructor.py +++ b/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""" diff --git a/test/unit/obj/test_reconstructor.py b/test/unit/obj/test_reconstructor.py index 2b6872ce4f..b7c9e83ce2 100644 --- a/test/unit/obj/test_reconstructor.py +++ b/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