diff --git a/swift/cli/manage_shard_ranges.py b/swift/cli/manage_shard_ranges.py index 508e7c1da2..8bfe01a9ae 100644 --- a/swift/cli/manage_shard_ranges.py +++ b/swift/cli/manage_shard_ranges.py @@ -948,6 +948,26 @@ def _add_prompt_args(parser): 'Cannot be used with --yes option.') +def _add_skip_or_force_commits_arg(parser): + """ + We merge in the pending file by default, this is always correct and + useful for probe tests where shard containers have unrealistically low + numbers of objects, of which a significant proportion may still be in the + pending file. If you have 10GB databases with 100M objects you can use + --skip-commits and the selected shard ranges probably won't be that + different. The --force-commits option is redundant and may be deprecated. + """ + group = parser.add_mutually_exclusive_group() + group.add_argument( + '--skip-commits', action='store_true', dest='skip_commits', + default=False, + help='Skip commits for pending object updates. By default the broker' + ' will commit pending object updates.') + group.add_argument( + '--force-commits', action='store_false', dest='skip_commits', + default=argparse.SUPPRESS, help=argparse.SUPPRESS) + + def _add_max_expanding_arg(parser): parser.add_argument( '--max-expanding', nargs='?', @@ -974,13 +994,8 @@ def _make_parser(): 'expansion_limit') parser.add_argument('--verbose', '-v', action='count', default=0, help='Increase output verbosity') - # this is useful for probe tests that shard containers with unrealistically - # low numbers of objects, of which a significant proportion may still be in - # the pending file - parser.add_argument( - '--force-commits', action='store_true', default=False, - help='Force broker to commit pending object updates before finding ' - 'shard ranges. By default the broker will skip commits.') + _add_skip_or_force_commits_arg(parser) + subparsers = parser.add_subparsers( dest='subcommand', help='Sub-command help', title='Sub-commands') @@ -1185,7 +1200,7 @@ def main(cli_args=None): logger = get_logger({}, name='ContainerBroker', log_to_console=True) broker = ContainerBroker(os.path.realpath(args.path_to_file), logger=logger, - skip_commits=not args.force_commits) + skip_commits=args.skip_commits) try: broker.get_info() except Exception as exc: diff --git a/test/unit/cli/test_manage_shard_ranges.py b/test/unit/cli/test_manage_shard_ranges.py index f4983375da..6da8a41441 100644 --- a/test/unit/cli/test_manage_shard_ranges.py +++ b/test/unit/cli/test_manage_shard_ranges.py @@ -160,7 +160,7 @@ class TestManageShardRanges(unittest.TestCase): func=mock.ANY, rows_per_shard=500000, subcommand='find', - force_commits=False, + skip_commits=False, verbose=0, minimum_shard_size=100000) mocked.assert_called_once_with(mock.ANY, expected) @@ -175,7 +175,7 @@ class TestManageShardRanges(unittest.TestCase): func=mock.ANY, rows_per_shard=600, subcommand='find', - force_commits=False, + skip_commits=False, verbose=0, minimum_shard_size=88) mocked.assert_called_once_with(mock.ANY, expected) @@ -191,7 +191,7 @@ class TestManageShardRanges(unittest.TestCase): func=mock.ANY, rows_per_shard=12345, subcommand='find', - force_commits=False, + skip_commits=False, verbose=0, minimum_shard_size=99) mocked.assert_called_once_with(mock.ANY, expected) @@ -205,7 +205,7 @@ class TestManageShardRanges(unittest.TestCase): path_to_file=mock.ANY, func=mock.ANY, subcommand='compact', - force_commits=False, + skip_commits=False, verbose=0, max_expanding=-1, max_shrinking=1, @@ -224,7 +224,7 @@ class TestManageShardRanges(unittest.TestCase): path_to_file=mock.ANY, func=mock.ANY, subcommand='compact', - force_commits=False, + skip_commits=False, verbose=0, max_expanding=31, max_shrinking=33, @@ -247,7 +247,7 @@ class TestManageShardRanges(unittest.TestCase): path_to_file=mock.ANY, func=mock.ANY, subcommand='compact', - force_commits=False, + skip_commits=False, verbose=0, max_expanding=11, max_shrinking=22, @@ -284,7 +284,7 @@ class TestManageShardRanges(unittest.TestCase): path_to_file=mock.ANY, func=mock.ANY, subcommand='compact', - force_commits=False, + skip_commits=False, verbose=0, max_expanding=31, max_shrinking=33, @@ -317,7 +317,7 @@ class TestManageShardRanges(unittest.TestCase): path_to_file=mock.ANY, func=mock.ANY, subcommand='compact', - force_commits=False, + skip_commits=False, verbose=0, max_expanding=31, max_shrinking=33, @@ -349,7 +349,7 @@ class TestManageShardRanges(unittest.TestCase): path_to_file=mock.ANY, func=mock.ANY, subcommand='compact', - force_commits=False, + skip_commits=False, verbose=0, max_expanding=31, max_shrinking=33, @@ -2928,3 +2928,31 @@ class TestManageShardRanges(unittest.TestCase): self.assertIn( "argument --yes/-y: not allowed with argument --dry-run/-n", err_lines[-2], err_lines) + + def test_skip_commits_and_force_commits_args(self): + broker = self._make_broker() + + with mock.patch('swift.cli.manage_shard_ranges.find_ranges', + return_value=0) as mocked: + ret = main([broker.db_file, '--skip-commits', 'find']) + self.assertEqual(0, ret) + args = mocked.call_args[0][1] + self.assertTrue(args.skip_commits, + '--skip-commits should set skip_commits to True') + + with mock.patch('swift.cli.manage_shard_ranges.find_ranges', + return_value=0) as mocked: + ret = main([broker.db_file, '--force-commits', 'find']) + self.assertEqual(0, ret) + args = mocked.call_args[0][1] + self.assertFalse(args.skip_commits, + '--force-commits should set skip_commits to False') + + # default (no flag) sets skip_commits to False + with mock.patch('swift.cli.manage_shard_ranges.find_ranges', + return_value=0) as mocked: + ret = main([broker.db_file, 'find']) + self.assertEqual(0, ret) + args = mocked.call_args[0][1] + self.assertFalse(args.skip_commits, + 'default should set skip_commits to False')