openstack/swift
Code Issues Proposed changes

sharder: add more validation checks on config

Browse Source 
Add checks that more of the sharder config values make sense with
respect to each other.

For the daemon, all config options are checked. For
swift-manage-shard-ranges only those options relevant to the current
subcommand are checked.

Change-Id: I2d4dab1d56c31f904161e5338f1bb89a03fd88f7
This commit is contained in:
Alistair Coles 2021-06-24 14:54:53 +01:00
parent 2a593174a5
commit 9b94c278f1
4 changed files with 118 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
swift/cli/manage_shard_ranges.py
View File

@ -934,6 +934,12 @@ def main(cli_args=None):
if v is USE_SHARDER_DEFAULT:
setattr(args, k, getattr(conf_args, k))
try:
ContainerSharderConf.validate_conf(args)
except ValueError as err:
print('Invalid config: %s' % err, file=sys.stderr)
return EXIT_INVALID_ARGS
if args.func in (analyze_shard_ranges,):
args.input = args.path_to_file
return args.func(args) or 0

30
swift/container/sharder.py
View File

@ -15,6 +15,7 @@
import collections
import errno
import json
import operator
import time
from collections import defaultdict
from operator import itemgetter
@ -634,18 +635,28 @@ class ContainerSharderConf(object):
'minimum_shard_size', config_positive_int_value,
max(self.rows_per_shard // 5, 1))
self.validate_conf()
def percent_of_threshold(self, val):
return int(config_percent_value(val) * self.shard_container_threshold)
def validate_conf(self):
keys = ('minimum_shard_size', 'rows_per_shard')
vals = [getattr(self, key) for key in keys]
if not vals[0] <= vals[1]:
raise ValueError(
'%s (%d) must be less than %s (%d)'
% (keys[0], vals[0], keys[1], vals[1]))
@classmethod
def validate_conf(cls, namespace):
ops = {'<': operator.lt,
'<=': operator.le}
checks = (('minimum_shard_size', '<=', 'rows_per_shard'),
('shrink_threshold', '<=', 'minimum_shard_size'),
('rows_per_shard', '<', 'shard_container_threshold'),
('expansion_limit', '<', 'shard_container_threshold'))
for key1, op, key2 in checks:
try:
val1 = getattr(namespace, key1)
val2 = getattr(namespace, key2)
except AttributeError:
# swift-manage-shard-ranges uses a subset of conf options for
# each command so only validate those actually in the namespace
continue
if not ops[op](val1, val2):
raise ValueError('%s (%d) must be %s %s (%d)'
% (key1, val1, op, key2, val2))
DEFAULT_SHARDER_CONF = vars(ContainerSharderConf())
@ -658,6 +669,7 @@ class ContainerSharder(ContainerSharderConf, ContainerReplicator):
logger = logger or get_logger(conf, log_route='container-sharder')
ContainerReplicator.__init__(self, conf, logger=logger)
ContainerSharderConf.__init__(self, conf)
ContainerSharderConf.validate_conf(self)
if conf.get('auto_create_account_prefix'):
self.logger.warning('Option auto_create_account_prefix is '
'deprecated. Configure '

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

@ -574,8 +574,8 @@ class TestManageShardRanges(unittest.TestCase):
'--minimum-shard-size', str(minimum)])
self.assertEqual(2, ret)
self.assertEqual(
'Error loading config: minimum_shard_size (%s) must be less '
'than rows_per_shard (50)' % minimum, err.getvalue().strip())
'Invalid config: minimum_shard_size (%s) must be <= '
'rows_per_shard (50)' % minimum, err.getvalue().strip())
assert_too_large_value_handled(51)
assert_too_large_value_handled(52)
@ -1461,7 +1461,7 @@ class TestManageShardRanges(unittest.TestCase):
with mock.patch('sys.stdout', out), mock.patch('sys.stderr', err):
ret = main([broker.db_file, 'compact', '--yes',
'--expansion-limit', '20'])
self.assertEqual(0, ret, out.getvalue())
self.assertEqual(0, ret, err.getvalue())
err_lines = err.getvalue().split('\n')
self.assert_starts_with(err_lines[0], 'Loaded db broker for ')
out_lines = out.getvalue().rstrip('\n').split('\n')

99
test/unit/container/test_sharder.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
import json
import random
from argparse import Namespace
import eventlet
import os
@ -210,7 +211,7 @@ class TestSharder(BaseTestSharder):
'rsync_compress': True,
'rsync_module': '{replication_ip}::container_sda/',
'reclaim_age': 86400 * 14,
'shrink_threshold': 7000000,
'shrink_threshold': 2000000,
'expansion_limit': 17000000,
'shard_container_threshold': 20000000,
'cleave_batch_size': 4,
@ -226,7 +227,7 @@ class TestSharder(BaseTestSharder):
'existing_shard_replication_quorum': 0,
'max_shrinking': 5,
'max_expanding': 4,
'rows_per_shard': 13
'rows_per_shard': 13000000
}
expected = {
'mount_check': False, 'bind_ip': '10.11.12.13', 'port': 62010,
@ -238,8 +239,8 @@ class TestSharder(BaseTestSharder):
'rsync_module': '{replication_ip}::container_sda',
'reclaim_age': 86400 * 14,
'shard_container_threshold': 20000000,
'rows_per_shard': 13,
'shrink_threshold': 7000000,
'rows_per_shard': 13000000,
'shrink_threshold': 2000000,
'expansion_limit': 17000000,
'cleave_batch_size': 4,
'shard_scanner_batch_size': 8,
@ -295,7 +296,7 @@ class TestSharder(BaseTestSharder):
def test_init_deprecated_options(self):
# percent values applied if absolute values not given
conf = {
'shard_shrink_point': 15, # trumps shrink_threshold
'shard_shrink_point': 7, # trumps shrink_threshold
'shard_shrink_merge_point': 95, # trumps expansion_limit
'shard_container_threshold': 20000000,
}
@ -310,7 +311,7 @@ class TestSharder(BaseTestSharder):
'reclaim_age': 86400 * 7,
'shard_container_threshold': 20000000,
'rows_per_shard': 10000000,
'shrink_threshold': 3000000,
'shrink_threshold': 1400000,
'expansion_limit': 19000000,
'cleave_batch_size': 2,
'shard_scanner_batch_size': 10,
@ -326,10 +327,10 @@ class TestSharder(BaseTestSharder):
self._do_test_init(conf, expected)
# absolute values override percent values
conf = {
'shard_shrink_point': 15, # trumps shrink_threshold
'shrink_threshold': 7000000,
'shard_shrink_merge_point': 95, # trumps expansion_limit
'expansion_limit': 17000000,
'shard_shrink_point': 7,
'shrink_threshold': 1300000, # trumps shard_shrink_point
'shard_shrink_merge_point': 95,
'expansion_limit': 17000000, # trumps shard_shrink_merge_point
'shard_container_threshold': 20000000,
}
expected = {
@ -343,7 +344,7 @@ class TestSharder(BaseTestSharder):
'reclaim_age': 86400 * 7,
'shard_container_threshold': 20000000,
'rows_per_shard': 10000000,
'shrink_threshold': 7000000,
'shrink_threshold': 1300000,
'expansion_limit': 17000000,
'cleave_batch_size': 2,
'shard_scanner_batch_size': 10,
@ -4223,6 +4224,7 @@ class TestSharder(BaseTestSharder):
account, cont, lower, upper)
with self._mock_sharder(conf={'shard_container_threshold': 199,
'minimum_shard_size': 1,
'shrink_threshold': 0,
'auto_create_account_prefix': '.int_'}
) as sharder:
with mock_timestamp_now() as now:
@ -4239,6 +4241,7 @@ class TestSharder(BaseTestSharder):
# second invocation finds none
with self._mock_sharder(conf={'shard_container_threshold': 199,
'minimum_shard_size': 1,
'shrink_threshold': 0,
'auto_create_account_prefix': '.int_'}
) as sharder:
num_found = sharder._find_shard_ranges(broker)
@ -4324,6 +4327,7 @@ class TestSharder(BaseTestSharder):
account, cont, lower, upper)
with self._mock_sharder(conf={'shard_container_threshold': 199,
'minimum_shard_size': 1,
'shrink_threshold': 0,
'auto_create_account_prefix': '.int_'}
) as sharder:
with mock_timestamp_now() as now:
@ -4412,6 +4416,7 @@ class TestSharder(BaseTestSharder):
# third invocation finds none
with self._mock_sharder(conf={'shard_container_threshold': 199,
'shard_scanner_batch_size': 2,
'shrink_threshold': 0,
'minimum_shard_size': 10}
) as sharder:
sharder._send_shard_ranges = mock.MagicMock(return_value=True)
@ -7412,3 +7417,75 @@ class TestContainerSharderConf(unittest.TestCase):
ValueError, msg='{%s : %s}' % (key, bad_value)) as cm:
ContainerSharderConf({key: bad_value})
self.assertIn('Error setting %s' % key, str(cm.exception))
def test_validate(self):
def assert_bad(conf):
with self.assertRaises(ValueError):
ContainerSharderConf.validate_conf(ContainerSharderConf(conf))
def assert_ok(conf):
try:
ContainerSharderConf.validate_conf(ContainerSharderConf(conf))
except ValueError as err:
self.fail('Unexpected ValueError: %s' % err)
assert_ok({})
assert_ok({'minimum_shard_size': 100,
'shrink_threshold': 100,
'rows_per_shard': 100})
assert_bad({'minimum_shard_size': 100})
assert_bad({'shrink_threshold': 100001})
assert_ok({'minimum_shard_size': 100,
'shrink_threshold': 100})
assert_bad({'minimum_shard_size': 100,
'shrink_threshold': 100,
'rows_per_shard': 99})
assert_ok({'shard_container_threshold': 100,
'rows_per_shard': 99})
assert_bad({'shard_container_threshold': 100,
'rows_per_shard': 100})
assert_bad({'rows_per_shard': 10000001})
assert_ok({'shard_container_threshold': 100,
'expansion_limit': 99})
assert_bad({'shard_container_threshold': 100,
'expansion_limit': 100})
assert_bad({'expansion_limit': 100000001})
def test_validate_subset(self):
# verify that validation is only applied for keys that exist in the
# given namespace
def assert_bad(conf):
with self.assertRaises(ValueError):
ContainerSharderConf.validate_conf(Namespace(**conf))
def assert_ok(conf):
try:
ContainerSharderConf.validate_conf(Namespace(**conf))
except ValueError as err:
self.fail('Unexpected ValueError: %s' % err)
assert_ok({})
assert_ok({'minimum_shard_size': 100,
'shrink_threshold': 100,
'rows_per_shard': 100})
assert_ok({'minimum_shard_size': 100})
assert_ok({'shrink_threshold': 100001})
assert_ok({'minimum_shard_size': 100,
'shrink_threshold': 100})
assert_bad({'minimum_shard_size': 100,
'shrink_threshold': 100,
'rows_per_shard': 99})
assert_ok({'shard_container_threshold': 100,
'rows_per_shard': 99})
assert_bad({'shard_container_threshold': 100,
'rows_per_shard': 100})
assert_ok({'rows_per_shard': 10000001})
assert_ok({'shard_container_threshold': 100,
'expansion_limit': 99})
assert_bad({'shard_container_threshold': 100,
'expansion_limit': 100})
assert_ok({'expansion_limit': 100000001})