openstack/swift
Code Issues Proposed changes

Merge "manage-shard-ranges: add --dry-run option for compact and repair"

Browse Source
This commit is contained in:
Zuul 2021-05-14 06:34:57 +00:00 committed by Gerrit Code Review
commit ea54a12aa7
2 changed files with 106 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
swift/cli/manage_shard_ranges.py
View File

@ -205,6 +205,20 @@ class InvalidSolutionException(ManageShardRangesException):
self.overlapping_donors = overlapping_donors 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): def _print_shard_range(sr, level=0):
indent = ' ' * level indent = ' ' * level
print(indent + '%r' % sr.name) print(indent + '%r' % sr.name)
@ -501,22 +515,20 @@ def compact_shard_ranges(broker, args):
file=sys.stderr) file=sys.stderr)
return EXIT_ERROR return EXIT_ERROR
if not args.yes: for sequence in compactible:
for sequence in compactible: acceptor = sequence[-1]
acceptor = sequence[-1] donors = sequence[:-1]
donors = sequence[:-1] print('Donor shard range(s) with total of %d rows:'
print('Donor shard range(s) with total of %d rows:' % donors.row_count)
% donors.row_count) for donor in donors:
for donor in donors: _print_shard_range(donor, level=1)
_print_shard_range(donor, level=1) print('can be compacted into acceptor shard range:')
print('can be compacted into acceptor shard range:') _print_shard_range(acceptor, level=1)
_print_shard_range(acceptor, level=1) print('Once applied to the broker these changes will result in shard '
print('Once applied to the broker these changes will result in shard ' 'range compaction the next time the sharder runs.')
'range compaction the next time the sharder runs.')
choice = input('Do you want to apply these changes? [yes/N]') if not _proceed(args):
if choice != 'yes': return EXIT_USER_QUIT
print('No changes applied')
return EXIT_USER_QUIT
process_compactible_shard_sequences(broker, compactible) process_compactible_shard_sequences(broker, compactible)
print('Updated %s shard sequences for compaction.' % len(compactible)) print('Updated %s shard sequences for compaction.' % len(compactible))
@ -651,12 +663,8 @@ def repair_shard_ranges(broker, args):
if not acceptor_path: if not acceptor_path:
return EXIT_SUCCESS return EXIT_SUCCESS
if not args.yes: if not _proceed(args):
choice = input('Do you want to apply these changes to the container ' return EXIT_USER_QUIT
'DB? [yes/N]')
if choice != 'yes':
print('No changes applied')
return EXIT_USER_QUIT
# merge changes to the broker... # merge changes to the broker...
# note: acceptors do not need to be modified since they already span the # 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.') help='DB timeout to use when enabling sharding.')
def _add_yes_arg(parser): def _add_prompt_args(parser):
parser.add_argument( parser.add_argument(
'--yes', '-y', action='store_true', default=False, '--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(): def _make_parser():
@ -808,7 +821,7 @@ def _make_parser():
'compact', 'compact',
help='Compact shard ranges with less than the shrink-threshold number ' help='Compact shard ranges with less than the shrink-threshold number '
'of rows. This command only works on root containers.') '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='?', compact_parser.add_argument('--shrink-threshold', nargs='?',
type=_positive_int, type=_positive_int,
default=None, default=None,
@ -849,7 +862,7 @@ def _make_parser():
'repair', 'repair',
help='Repair overlapping shard ranges. No action will be taken ' help='Repair overlapping shard ranges. No action will be taken '
'without user confirmation unless the -y option is used.') '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) repair_parser.set_defaults(func=repair_shard_ranges)
# analyze # analyze
@ -875,6 +888,10 @@ def main(args=None):
print('\nA sub-command is required.', file=sys.stderr) print('\nA sub-command is required.', file=sys.stderr)
return EXIT_INVALID_ARGS 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 = {} conf = {}
rows_per_shard = DEFAULT_ROWS_PER_SHARD rows_per_shard = DEFAULT_ROWS_PER_SHARD
shrink_threshold = DEFAULT_SHRINK_THRESHOLD shrink_threshold = DEFAULT_SHRINK_THRESHOLD

101
test/unit/cli/test_manage_shard_ranges.py
View File

@ -203,7 +203,8 @@ class TestManageShardRanges(unittest.TestCase):
max_shrinking=1, max_shrinking=1,
shrink_threshold=100000, shrink_threshold=100000,
expansion_limit=500000, expansion_limit=500000,
yes=False) yes=False,
dry_run=False)
mocked.assert_called_once_with(mock.ANY, expected) mocked.assert_called_once_with(mock.ANY, expected)
# conf file # conf file
@ -221,7 +222,8 @@ class TestManageShardRanges(unittest.TestCase):
max_shrinking=33, max_shrinking=33,
shrink_threshold=150, shrink_threshold=150,
expansion_limit=650, expansion_limit=650,
yes=False) yes=False,
dry_run=False)
mocked.assert_called_once_with(mock.ANY, expected) mocked.assert_called_once_with(mock.ANY, expected)
# conf file - small percentages resulting in zero absolute values # conf file - small percentages resulting in zero absolute values
@ -253,7 +255,8 @@ class TestManageShardRanges(unittest.TestCase):
max_shrinking=33, max_shrinking=33,
shrink_threshold=0, shrink_threshold=0,
expansion_limit=0, expansion_limit=0,
yes=False) yes=False,
dry_run=False)
mocked.assert_called_once_with(mock.ANY, expected) mocked.assert_called_once_with(mock.ANY, expected)
# cli options # cli options
@ -275,7 +278,8 @@ class TestManageShardRanges(unittest.TestCase):
max_shrinking=22, max_shrinking=22,
shrink_threshold=1234, shrink_threshold=1234,
expansion_limit=3456, expansion_limit=3456,
yes=False) yes=False,
dry_run=False)
mocked.assert_called_once_with(mock.ANY, expected) mocked.assert_called_once_with(mock.ANY, expected)
# conf file - invalid value for shard_container_threshold # 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.', '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() out = StringIO()
err = StringIO() err = StringIO()
with mock.patch('sys.stdout', out),\ with mock.patch('sys.stdout', out),\
@ -902,13 +906,13 @@ class TestManageShardRanges(unittest.TestCase):
mock.patch('swift.cli.manage_shard_ranges.input', mock.patch('swift.cli.manage_shard_ranges.input',
return_value=user_input): return_value=user_input):
ret = main([broker.db_file, 'compact', ret = main([broker.db_file, 'compact',
'--max-shrinking', '99']) '--max-shrinking', '99'] + options)
self.assertEqual(exit_code, ret) self.assertEqual(exit_code, ret)
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
expected = list(expected_base) expected = list(expected_base)
if user_input == 'yes': if exp_changes:
expected.extend([ expected.extend([
'Updated 2 shard sequences for compaction.', 'Updated 2 shard sequences for compaction.',
'Run container-replicator to replicate the changes to ' '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]) self.assertEqual(expected, [l.split('/', 1)[0] for l in out_lines])
return broker.get_shard_ranges() return broker.get_shard_ranges()
broker_ranges = do_compact('n', 3) broker_ranges = do_compact('n', [], False, 3)
# expect no changes to shard ranges # expect no changes to shard ranges
self.assertEqual(shard_ranges, broker_ranges) self.assertEqual(shard_ranges, broker_ranges)
for i, sr in enumerate(broker_ranges): for i, sr in enumerate(broker_ranges):
self.assertEqual(ShardRange.ACTIVE, sr.state) 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 # expect updated shard ranges
shard_ranges[5].lower = shard_ranges[3].lower shard_ranges[5].lower = shard_ranges[3].lower
shard_ranges[8].lower = shard_ranges[7].lower shard_ranges[8].lower = shard_ranges[7].lower
@ -960,9 +970,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual( self.assertIn('Updated 2 shard sequences for compaction.', out_lines)
['Updated 2 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
for i, sr in enumerate(updated_ranges): for i, sr in enumerate(updated_ranges):
if i in small_ranges: if i in small_ranges:
@ -1013,9 +1021,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual( self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
['Updated 1 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
self.assertEqual(shard_ranges, updated_ranges) self.assertEqual(shard_ranges, updated_ranges)
self.assertEqual([ShardRange.SHRINKING] * 10, self.assertEqual([ShardRange.SHRINKING] * 10,
@ -1052,9 +1058,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual( self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
['Updated 1 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
self.assertEqual(shard_ranges, updated_ranges) self.assertEqual(shard_ranges, updated_ranges)
self.assertEqual([ShardRange.SHRINKING], self.assertEqual([ShardRange.SHRINKING],
@ -1093,9 +1097,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual( self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
['Updated 1 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
shard_ranges[9].lower = shard_ranges[4].lower # expanded acceptor shard_ranges[9].lower = shard_ranges[4].lower # expanded acceptor
self.assertEqual(shard_ranges, updated_ranges) self.assertEqual(shard_ranges, updated_ranges)
@ -1126,9 +1128,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual( self.assertIn('Updated 2 shard sequences for compaction.', out_lines)
['Updated 2 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
gapped_ranges[2].lower = gapped_ranges[0].lower gapped_ranges[2].lower = gapped_ranges[0].lower
gapped_ranges[8].lower = gapped_ranges[3].lower gapped_ranges[8].lower = gapped_ranges[3].lower
@ -1155,7 +1155,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual([expect_msg], out_lines[:1]) self.assertIn(expect_msg, out_lines)
return broker.get_shard_ranges() return broker.get_shard_ranges()
updated_ranges = do_compact( updated_ranges = do_compact(
@ -1191,7 +1191,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual([expect_msg], out_lines[:1]) self.assertIn(expect_msg, out_lines)
return broker.get_shard_ranges() return broker.get_shard_ranges()
updated_ranges = do_compact( updated_ranges = do_compact(
@ -1231,7 +1231,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual([expect_msg], out_lines[:1]) self.assertIn(expect_msg, out_lines)
return broker.get_shard_ranges() return broker.get_shard_ranges()
updated_ranges = do_compact( updated_ranges = do_compact(
@ -1267,10 +1267,8 @@ class TestManageShardRanges(unittest.TestCase):
self.assertEqual(0, ret, out.getvalue()) self.assertEqual(0, ret, out.getvalue())
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().rstrip('\n').split('\n')
self.assertEqual( self.assertIn('Updated 5 shard sequences for compaction.', out_lines)
['Updated 5 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
shard_ranges[1].lower = shard_ranges[0].lower shard_ranges[1].lower = shard_ranges[0].lower
shard_ranges[3].lower = shard_ranges[2].lower shard_ranges[3].lower = shard_ranges[2].lower
@ -1378,9 +1376,7 @@ class TestManageShardRanges(unittest.TestCase):
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().split('\n') out_lines = out.getvalue().split('\n')
self.assertEqual( self.assertIn('Updated 1 shard sequences for compaction.', out_lines)
['Updated 1 shard sequences for compaction.'],
out_lines[:1])
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
shard_ranges[8].lower = shard_ranges[0].lower shard_ranges[8].lower = shard_ranges[0].lower
self.assertEqual(shard_ranges, updated_ranges) self.assertEqual(shard_ranges, updated_ranges)
@ -1565,7 +1561,8 @@ class TestManageShardRanges(unittest.TestCase):
broker.merge_shard_ranges(shard_ranges + overlap_shard_ranges_2) broker.merge_shard_ranges(shard_ranges + overlap_shard_ranges_2)
self.assertTrue(broker.is_root_container()) 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() out = StringIO()
err = StringIO() err = StringIO()
with mock.patch('sys.stdout', out), \ with mock.patch('sys.stdout', out), \
@ -1573,7 +1570,7 @@ class TestManageShardRanges(unittest.TestCase):
mock_timestamp_now(ts_now), \ mock_timestamp_now(ts_now), \
mock.patch('swift.cli.manage_shard_ranges.input', mock.patch('swift.cli.manage_shard_ranges.input',
return_value=user_input): return_value=user_input):
ret = main([broker.db_file, 'repair']) ret = main([broker.db_file, 'repair'] + options)
self.assertEqual(exit_code, ret) self.assertEqual(exit_code, ret)
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ') self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
@ -1584,7 +1581,25 @@ class TestManageShardRanges(unittest.TestCase):
# user input 'n' # user input 'n'
ts_now = next(self.ts_iter) 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() updated_ranges = broker.get_shard_ranges()
expected = sorted( expected = sorted(
shard_ranges + overlap_shard_ranges_2, shard_ranges + overlap_shard_ranges_2,
@ -1593,7 +1608,7 @@ class TestManageShardRanges(unittest.TestCase):
# user input 'yes' # user input 'yes'
ts_now = next(self.ts_iter) ts_now = next(self.ts_iter)
do_repair('yes', ts_now, 0) do_repair('yes', ts_now, [], 0)
updated_ranges = broker.get_shard_ranges() updated_ranges = broker.get_shard_ranges()
for sr in overlap_shard_ranges_2: for sr in overlap_shard_ranges_2:
sr.update_state(ShardRange.SHRINKING, ts_now) sr.update_state(ShardRange.SHRINKING, ts_now)
@ -1755,3 +1770,13 @@ class TestManageShardRanges(unittest.TestCase):
self.assertEqual(2, ret) self.assertEqual(2, ret)
err_lines = err.getvalue().split('\n') err_lines = err.getvalue().split('\n')
self.assertIn('A sub-command is required.', err_lines) 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)