From f140de75e4e63ae0d101a2903a737e5727e7d794 Mon Sep 17 00:00:00 2001 From: Matthew Oliver Date: Fri, 7 May 2021 12:11:25 +1000 Subject: [PATCH] relinker: let --workers accept auto Currently from the relinker cli we allow a --workers parameter but it currently _must_ be an int. However, if someone wants to override a static int worker value from the config back to the 'auto' this results in a VALUE error. This patch changes the argparse type check for the worker param to use config_auto_int_value, via a small shim, so a value of 'auto' will be accepted. Closes-Bug: #1926757 Change-Id: I5691bf644ade9fd7f7490f91add388a1d5a4aaa2 --- swift/cli/relinker.py | 6 ++- test/unit/cli/test_relinker.py | 78 +++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py index 042398dc93..d65d684ec5 100644 --- a/swift/cli/relinker.py +++ b/swift/cli/relinker.py @@ -603,6 +603,10 @@ def parallel_process(do_cleanup, conf, logger=None, device_list=None): return final_status +def auto_or_int(value): + return config_auto_int_value(value, default='auto') + + def main(args): parser = argparse.ArgumentParser( description='Relink and cleanup objects to increase partition power') @@ -633,7 +637,7 @@ def main(args): help='Used to limit I/O. Zero implies no limit ' '(default: no limit).') parser.add_argument( - '--workers', default=None, type=non_negative_int, help=( + '--workers', default=None, type=auto_or_int, help=( 'Process devices across N workers ' '(default: one worker per device)')) parser.add_argument('--logfile', default=None, dest='logfile', diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py index ff118d29f8..ca47cdf927 100644 --- a/test/unit/cli/test_relinker.py +++ b/test/unit/cli/test_relinker.py @@ -520,7 +520,7 @@ class TestRelinker(unittest.TestCase): '--skip-mount-check', '--files-per-second', '2.2', '--policy', '1', '--partition', '123', '--partition', '123', '--partition', '456', - '--link-check-limit', '3', + '--link-check-limit', '3', '--workers', '2' ]) mock_relinker.assert_called_once_with({ '__file__': mock.ANY, @@ -532,7 +532,7 @@ class TestRelinker(unittest.TestCase): 'log_name': 'test-relinker', 'policies': {POLICIES[1]}, 'partitions': {123, 456}, - 'workers': 'auto', + 'workers': 2, 'link_check_limit': 3, }, mock.ANY, ['sdx'], do_cleanup=False) @@ -582,6 +582,80 @@ class TestRelinker(unittest.TestCase): mock_logging_config.assert_called_once_with( format='%(message)s', level=logging.DEBUG, filename=None) + # now test overriding workers back to auto + config = """ + [DEFAULT] + swift_dir = test/swift/dir + devices = /test/node + mount_check = true + + [object-relinker] + log_level = WARNING + log_name = test-relinker + files_per_second = 11.1 + link_check_limit = 1 + workers = 8 + """ + with open(conf_file, 'w') as f: + f.write(dedent(config)) + devices = ['sdx%d' % i for i in range(8, 1)] + cli_cmd = ['relink', conf_file, '--device', 'sdx', '--workers', 'auto'] + for device in devices: + cli_cmd.extend(['--device', device]) + with mock.patch('swift.cli.relinker.Relinker') as mock_relinker: + relinker.main(cli_cmd) + mock_relinker.assert_called_once_with({ + '__file__': mock.ANY, + 'swift_dir': 'test/swift/dir', + 'devices': '/test/node', + 'mount_check': True, + 'files_per_second': 11.1, + 'log_name': 'test-relinker', + 'log_level': 'WARNING', + 'policies': POLICIES, + 'partitions': set(), + 'workers': 'auto', + 'link_check_limit': 1, + }, mock.ANY, ['sdx'], do_cleanup=False) + logger = mock_relinker.call_args[0][1] + self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) + self.assertEqual('test-relinker', logger.logger.name) + + # and now globally + config = """ + [DEFAULT] + swift_dir = test/swift/dir + devices = /test/node + mount_check = true + workers = 8 + + [object-relinker] + log_level = WARNING + log_name = test-relinker + files_per_second = 11.1 + link_check_limit = 1 + """ + with open(conf_file, 'w') as f: + f.write(dedent(config)) + with mock.patch('swift.cli.relinker.Relinker') as mock_relinker: + relinker.main(cli_cmd) + mock_relinker.assert_called_once_with({ + '__file__': mock.ANY, + 'swift_dir': 'test/swift/dir', + 'devices': '/test/node', + 'mount_check': True, + 'files_per_second': 11.1, + 'log_name': 'test-relinker', + 'log_level': 'WARNING', + 'policies': POLICIES, + 'partitions': set(), + 'workers': 'auto', + 'link_check_limit': 1, + }, mock.ANY, ['sdx'], do_cleanup=False) + logger = mock_relinker.call_args[0][1] + self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) + self.assertEqual('test-relinker', logger.logger.name) + def test_relink_first_quartile_no_rehash(self): # we need object name in lower half of current part self._setup_object(lambda part: part < 2 ** (PART_POWER - 1))