manage-shard-ranges: add --dry-run option for compact and repair
Change-Id: Ib566a2618abd78aa08a9b540000b2094e7f0e25e
This commit is contained in:
Alistair Coles
parent
4ce907a4ae
commit
568036f1b7
|
|
@ -205,6 +205,20 @@ class InvalidSolutionException(ManageShardRangesException):
|
|||
self.overlapping_donors = overlapping_donors
|
||||
|
||||
|
||||
def _proceed(args):
|
||||
if args.dry_run:
|
||||
choice = 'no'
|
||||
elif args.yes:
|
||||
choice = 'yes'
|
||||
else:
|
||||
choice = input('Do you want to apply these changes to the container '
|
||||
'DB? [yes/N]')
|
||||
if choice != 'yes':
|
||||
print('No changes applied')
|
||||
|
||||
return choice == 'yes'
|
||||
|
||||
|
||||
def _print_shard_range(sr, level=0):
|
||||
indent = ' ' * level
|
||||
print(indent + '%r' % sr.name)
|
||||
|
|
@ -501,22 +515,20 @@ def compact_shard_ranges(broker, args):
|
|||
file=sys.stderr)
|
||||
return EXIT_ERROR
|
||||
|
||||
if not args.yes:
|
||||
for sequence in compactible:
|
||||
acceptor = sequence[-1]
|
||||
donors = sequence[:-1]
|
||||
print('Donor shard range(s) with total of %d rows:'
|
||||
% donors.row_count)
|
||||
for donor in donors:
|
||||
_print_shard_range(donor, level=1)
|
||||
print('can be compacted into acceptor shard range:')
|
||||
_print_shard_range(acceptor, level=1)
|
||||
print('Once applied to the broker these changes will result in shard '
|
||||
'range compaction the next time the sharder runs.')
|
||||
choice = input('Do you want to apply these changes? [yes/N]')
|
||||
if choice != 'yes':
|
||||
print('No changes applied')
|
||||
return EXIT_USER_QUIT
|
||||
for sequence in compactible:
|
||||
acceptor = sequence[-1]
|
||||
donors = sequence[:-1]
|
||||
print('Donor shard range(s) with total of %d rows:'
|
||||
% donors.row_count)
|
||||
for donor in donors:
|
||||
_print_shard_range(donor, level=1)
|
||||
print('can be compacted into acceptor shard range:')
|
||||
_print_shard_range(acceptor, level=1)
|
||||
print('Once applied to the broker these changes will result in shard '
|
||||
'range compaction the next time the sharder runs.')
|
||||
|
||||
if not _proceed(args):
|
||||
return EXIT_USER_QUIT
|
||||
|
||||
process_compactible_shard_sequences(broker, compactible)
|
||||
print('Updated %s shard sequences for compaction.' % len(compactible))
|
||||
|
|
@ -651,12 +663,8 @@ def repair_shard_ranges(broker, args):
|
|||
if not acceptor_path:
|
||||
return EXIT_SUCCESS
|
||||
|
||||
if not args.yes:
|
||||
choice = input('Do you want to apply these changes to the container '
|
||||
'DB? [yes/N]')
|
||||
if choice != 'yes':
|
||||
print('No changes applied')
|
||||
return EXIT_USER_QUIT
|
||||
if not _proceed(args):
|
||||
return EXIT_USER_QUIT
|
||||
|
||||
# merge changes to the broker...
|
||||
# note: acceptors do not need to be modified since they already span the
|
||||
|
|
@ -717,10 +725,15 @@ def _add_enable_args(parser):
|
|||
help='DB timeout to use when enabling sharding.')
|
||||
|
||||
|
||||
def _add_yes_arg(parser):
|
||||
def _add_prompt_args(parser):
|
||||
parser.add_argument(
|
||||
'--yes', '-y', action='store_true', default=False,
|
||||
help='Apply shard range changes to broker without prompting.')
|
||||
help='Apply shard range changes to broker without prompting. '
|
||||
'Cannot be used with --dry-run option.')
|
||||
parser.add_argument(
|
||||
'--dry-run', '-n', action='store_true', default=False,
|
||||
help='Do not apply any shard range changes to broker. '
|
||||
'Cannot be used with --yes option.')
|
||||
|
||||
|
||||
def _make_parser():
|
||||
|
|
@ -808,7 +821,7 @@ def _make_parser():
|
|||
'compact',
|
||||
help='Compact shard ranges with less than the shrink-threshold number '
|
||||
'of rows. This command only works on root containers.')
|
||||
_add_yes_arg(compact_parser)
|
||||
_add_prompt_args(compact_parser)
|
||||
compact_parser.add_argument('--shrink-threshold', nargs='?',
|
||||
type=_positive_int,
|
||||
default=None,
|
||||
|
|
@ -849,7 +862,7 @@ def _make_parser():
|
|||
'repair',
|
||||
help='Repair overlapping shard ranges. No action will be taken '
|
||||
'without user confirmation unless the -y option is used.')
|
||||
_add_yes_arg(repair_parser)
|
||||
_add_prompt_args(repair_parser)
|
||||
repair_parser.set_defaults(func=repair_shard_ranges)
|
||||
|
||||
# analyze
|
||||
|
|
@ -875,6 +888,10 @@ def main(args=None):
|
|||
print('\nA sub-command is required.', file=sys.stderr)
|
||||
return EXIT_INVALID_ARGS
|
||||
|
||||
if getattr(args, 'yes', False) and getattr(args, 'dry_run', False):
|
||||
print('--yes and --dry-run cannot both be set.', file=sys.stderr)
|
||||
return EXIT_INVALID_ARGS
|
||||
|
||||
conf = {}
|
||||
rows_per_shard = DEFAULT_ROWS_PER_SHARD
|
||||
shrink_threshold = DEFAULT_SHRINK_THRESHOLD
|
||||
|
|
|
|||
|
|
@ -203,7 +203,8 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
max_shrinking=1,
|
||||
shrink_threshold=100000,
|
||||
expansion_limit=500000,
|
||||
yes=False)
|
||||
yes=False,
|
||||
dry_run=False)
|
||||
mocked.assert_called_once_with(mock.ANY, expected)
|
||||
|
||||
# conf file
|
||||
|
|
@ -221,7 +222,8 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
max_shrinking=33,
|
||||
shrink_threshold=150,
|
||||
expansion_limit=650,
|
||||
yes=False)
|
||||
yes=False,
|
||||
dry_run=False)
|
||||
mocked.assert_called_once_with(mock.ANY, expected)
|
||||
|
||||
# conf file - small percentages resulting in zero absolute values
|
||||
|
|
@ -253,7 +255,8 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
max_shrinking=33,
|
||||
shrink_threshold=0,
|
||||
expansion_limit=0,
|
||||
yes=False)
|
||||
yes=False,
|
||||
dry_run=False)
|
||||
mocked.assert_called_once_with(mock.ANY, expected)
|
||||
|
||||
# cli options
|
||||
|
|
@ -275,7 +278,8 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
max_shrinking=22,
|
||||
shrink_threshold=1234,
|
||||
expansion_limit=3456,
|
||||
yes=False)
|
||||
yes=False,
|
||||
dry_run=False)
|
||||
mocked.assert_called_once_with(mock.ANY, expected)
|
||||
|
||||
# conf file - invalid value for shard_container_threshold
|
||||
|
|
@ -894,7 +898,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
'shard range compaction the next time the sharder runs.',
|
||||
]
|
||||
|
||||
def do_compact(user_input, exit_code):
|
||||
def do_compact(user_input, options, exp_changes, exit_code):
|
||||
out = StringIO()
|
||||
err = StringIO()
|
||||
with mock.patch('sys.stdout', out),\
|
||||
|
|
@ -902,13 +906,13 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
mock.patch('swift.cli.manage_shard_ranges.input',
|
||||
return_value=user_input):
|
||||
ret = main([broker.db_file, 'compact',
|
||||
'--max-shrinking', '99'])
|
||||
'--max-shrinking', '99'] + options)
|
||||
self.assertEqual(exit_code, ret)
|
||||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
expected = list(expected_base)
|
||||
if user_input == 'yes':
|
||||
if exp_changes:
|
||||
expected.extend([
|
||||
'Updated 2 shard sequences for compaction.',
|
||||
'Run container-replicator to replicate the changes to '
|
||||
|
|
@ -924,13 +928,19 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
self.assertEqual(expected, [l.split('/', 1)[0] for l in out_lines])
|
||||
return broker.get_shard_ranges()
|
||||
|
||||
broker_ranges = do_compact('n', 3)
|
||||
broker_ranges = do_compact('n', [], False, 3)
|
||||
# expect no changes to shard ranges
|
||||
self.assertEqual(shard_ranges, broker_ranges)
|
||||
for i, sr in enumerate(broker_ranges):
|
||||
self.assertEqual(ShardRange.ACTIVE, sr.state)
|
||||
|
||||
broker_ranges = do_compact('yes', 0)
|
||||
broker_ranges = do_compact('yes', ['--dry-run'], False, 3)
|
||||
# expect no changes to shard ranges
|
||||
self.assertEqual(shard_ranges, broker_ranges)
|
||||
for i, sr in enumerate(broker_ranges):
|
||||
self.assertEqual(ShardRange.ACTIVE, sr.state)
|
||||
|
||||
broker_ranges = do_compact('yes', [], True, 0)
|
||||
# expect updated shard ranges
|
||||
shard_ranges[5].lower = shard_ranges[3].lower
|
||||
shard_ranges[8].lower = shard_ranges[7].lower
|
||||
|
|
@ -960,9 +970,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 2 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
self.assertIn('Updated 2 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
for i, sr in enumerate(updated_ranges):
|
||||
if i in small_ranges:
|
||||
|
|
@ -1013,9 +1021,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 1 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
self.assertEqual(shard_ranges, updated_ranges)
|
||||
self.assertEqual([ShardRange.SHRINKING] * 10,
|
||||
|
|
@ -1052,9 +1058,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 1 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
self.assertEqual(shard_ranges, updated_ranges)
|
||||
self.assertEqual([ShardRange.SHRINKING],
|
||||
|
|
@ -1093,9 +1097,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 1 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
shard_ranges[9].lower = shard_ranges[4].lower # expanded acceptor
|
||||
self.assertEqual(shard_ranges, updated_ranges)
|
||||
|
|
@ -1126,9 +1128,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 2 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
self.assertIn('Updated 2 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
gapped_ranges[2].lower = gapped_ranges[0].lower
|
||||
gapped_ranges[8].lower = gapped_ranges[3].lower
|
||||
|
|
@ -1155,7 +1155,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual([expect_msg], out_lines[:1])
|
||||
self.assertIn(expect_msg, out_lines)
|
||||
return broker.get_shard_ranges()
|
||||
|
||||
updated_ranges = do_compact(
|
||||
|
|
@ -1191,7 +1191,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual([expect_msg], out_lines[:1])
|
||||
self.assertIn(expect_msg, out_lines)
|
||||
return broker.get_shard_ranges()
|
||||
|
||||
updated_ranges = do_compact(
|
||||
|
|
@ -1231,7 +1231,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual([expect_msg], out_lines[:1])
|
||||
self.assertIn(expect_msg, out_lines)
|
||||
return broker.get_shard_ranges()
|
||||
|
||||
updated_ranges = do_compact(
|
||||
|
|
@ -1267,10 +1267,8 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
self.assertEqual(0, ret, out.getvalue())
|
||||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 5 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
out_lines = out.getvalue().rstrip('\n').split('\n')
|
||||
self.assertIn('Updated 5 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
shard_ranges[1].lower = shard_ranges[0].lower
|
||||
shard_ranges[3].lower = shard_ranges[2].lower
|
||||
|
|
@ -1378,9 +1376,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
out_lines = out.getvalue().split('\n')
|
||||
self.assertEqual(
|
||||
['Updated 1 shard sequences for compaction.'],
|
||||
out_lines[:1])
|
||||
self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
shard_ranges[8].lower = shard_ranges[0].lower
|
||||
self.assertEqual(shard_ranges, updated_ranges)
|
||||
|
|
@ -1565,7 +1561,8 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
broker.merge_shard_ranges(shard_ranges + overlap_shard_ranges_2)
|
||||
self.assertTrue(broker.is_root_container())
|
||||
|
||||
def do_repair(user_input, ts_now, exit_code):
|
||||
def do_repair(user_input, ts_now, options, exit_code):
|
||||
options = options if options else []
|
||||
out = StringIO()
|
||||
err = StringIO()
|
||||
with mock.patch('sys.stdout', out), \
|
||||
|
|
@ -1573,7 +1570,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
mock_timestamp_now(ts_now), \
|
||||
mock.patch('swift.cli.manage_shard_ranges.input',
|
||||
return_value=user_input):
|
||||
ret = main([broker.db_file, 'repair'])
|
||||
ret = main([broker.db_file, 'repair'] + options)
|
||||
self.assertEqual(exit_code, ret)
|
||||
err_lines = err.getvalue().split('\n')
|
||||
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
|
||||
|
|
@ -1584,7 +1581,25 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
|
||||
# user input 'n'
|
||||
ts_now = next(self.ts_iter)
|
||||
do_repair('n', ts_now, 3)
|
||||
do_repair('n', ts_now, [], 3)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
expected = sorted(
|
||||
shard_ranges + overlap_shard_ranges_2,
|
||||
key=ShardRange.sort_key)
|
||||
self.assert_shard_ranges_equal(expected, updated_ranges)
|
||||
|
||||
# --dry-run
|
||||
ts_now = next(self.ts_iter)
|
||||
do_repair('y', ts_now, ['--dry-run'], 3)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
expected = sorted(
|
||||
shard_ranges + overlap_shard_ranges_2,
|
||||
key=ShardRange.sort_key)
|
||||
self.assert_shard_ranges_equal(expected, updated_ranges)
|
||||
|
||||
# --n
|
||||
ts_now = next(self.ts_iter)
|
||||
do_repair('y', ts_now, ['-n'], 3)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
expected = sorted(
|
||||
shard_ranges + overlap_shard_ranges_2,
|
||||
|
|
@ -1593,7 +1608,7 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
|
||||
# user input 'yes'
|
||||
ts_now = next(self.ts_iter)
|
||||
do_repair('yes', ts_now, 0)
|
||||
do_repair('yes', ts_now, [], 0)
|
||||
updated_ranges = broker.get_shard_ranges()
|
||||
for sr in overlap_shard_ranges_2:
|
||||
sr.update_state(ShardRange.SHRINKING, ts_now)
|
||||
|
|
@ -1755,3 +1770,13 @@ class TestManageShardRanges(unittest.TestCase):
|
|||
self.assertEqual(2, ret)
|
||||
err_lines = err.getvalue().split('\n')
|
||||
self.assertIn('A sub-command is required.', err_lines)
|
||||
|
||||
def test_dry_run_and_yes_is_invalid(self):
|
||||
out = StringIO()
|
||||
err = StringIO()
|
||||
with mock.patch('sys.stdout', out), \
|
||||
mock.patch('sys.stderr', err):
|
||||
ret = main(['db file', 'repair', '--dry-run', '--yes'])
|
||||
self.assertEqual(2, ret)
|
||||
err_lines = err.getvalue().split('\n')
|
||||
self.assertIn('--yes and --dry-run cannot both be set.', err_lines)
|
||||
|
|
|
|||
Loading…
Reference in New Issue