From 3517ca453e0f1994cdbde69b2c4e1764c91586f2 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Wed, 4 May 2022 22:13:52 +0100 Subject: [PATCH] backend ratelimit: support per-method rate limits Add support for config options such as: head_requests_per_device_per_second = 100 Change-Id: I2936f799b6112155ff01dcd8e1f985849a1af178 --- etc/backend-ratelimit.conf-sample | 30 +- swift/common/middleware/backend_ratelimit.py | 110 +++-- .../middleware/test_backend_ratelimit.py | 464 +++++++++++++++--- 3 files changed, 485 insertions(+), 119 deletions(-) diff --git a/etc/backend-ratelimit.conf-sample b/etc/backend-ratelimit.conf-sample index 4c38c20a2f..5912b6668e 100644 --- a/etc/backend-ratelimit.conf-sample +++ b/etc/backend-ratelimit.conf-sample @@ -1,10 +1,30 @@ [backend_ratelimit] -# Set the maximum rate of requests per second per device per worker. Beyond -# this rate the server will return 529 responses and emit a 'backend.ratelimit' -# statsd metric without logging. The default value of zero causes no -# rate-limiting to be applied. +# The rate of requests to each device is limited by an overall per-device rate +# limit that is applied to all requests to the device and/or a +# per-method-per-device rate limit that is applied to requests of that method +# to the device. If either of these rates would be exceeded the server will +# return 529 responses and emit a 'backend.ratelimit' statsd metric without +# logging. + +# Set the maximum overall rate of requests per device per second per worker for +# all request methods. The default value of zero causes no per-device +# rate-limiting to be applied other than that configured for specific request +# methods. # requests_per_device_per_second = 0.0 -# + +# Set maximum rate of requests per device per second per worker for individual +# request methods. The default value of zero causes no per-method +# rate-limiting to be applied. Note: the aggregate rate of requests for all +# methods is still limited by requests_per_device_per_second even if a higher +# per method rate is configured. +# delete_requests_per_device_per_second = 0.0 +# get_requests_per_device_per_second = 0.0 +# head_requests_per_device_per_second = 0.0 +# post_requests_per_device_per_second = 0.0 +# put_requests_per_device_per_second = 0.0 +# replicate_requests_per_device_per_second = 0.0 +# update_requests_per_device_per_second = 0.0 + # Set the number of seconds of unused rate-limiting allowance that can # accumulate and be used to allow a subsequent burst of requests. # requests_per_device_rate_buffer = 1.0 diff --git a/swift/common/middleware/backend_ratelimit.py b/swift/common/middleware/backend_ratelimit.py index a9e6e8925f..696d926ae8 100644 --- a/swift/common/middleware/backend_ratelimit.py +++ b/swift/common/middleware/backend_ratelimit.py @@ -14,7 +14,6 @@ # limitations under the License. import os import time -from collections import defaultdict from swift.common.request_helpers import split_and_validate_path from swift.common.swob import Request, HTTPTooManyBackendRequests, \ @@ -35,31 +34,26 @@ class BackendRateLimitMiddleware(object): """ Backend rate-limiting middleware. - Rate-limits requests to backend storage node devices. Each device is - independently rate-limited. All requests with a 'GET', 'HEAD', 'PUT', - 'POST', 'DELETE', 'UPDATE' or 'REPLICATE' method are included in a device's - rate limit. + Rate-limits requests to backend storage node devices. Each (device, request + method) combination is independently rate-limited. All requests with a + 'GET', 'HEAD', 'PUT', 'POST', 'DELETE', 'UPDATE' or 'REPLICATE' method are + rate limited on a per-device basis by both a method-specific rate and an + overall device rate limit. - If a request would cause the rate-limit to be exceeded then a response with - a 529 status code is returned. + If a request would cause the rate-limit to be exceeded for the method + and/or device then a response with a 529 status code is returned. """ def __init__(self, app, filter_conf, logger=None): self.app = app self.filter_conf = filter_conf - self.current_conf = {} self.logger = logger or get_logger(self.filter_conf, log_route='backend_ratelimit') - self.requests_per_device_per_second = \ - DEFAULT_REQUESTS_PER_DEVICE_PER_SECOND self.requests_per_device_rate_buffer = \ DEFAULT_REQUESTS_PER_DEVICE_RATE_BUFFER - # map device -> RateLimiter - self.rate_limiters = defaultdict( - lambda: EventletRateLimiter( - max_rate=self.requests_per_device_per_second, - rate_buffer=self.requests_per_device_rate_buffer, - running_time=time.time(), - burst_after_idle=True)) + # map (device, method) -> rate + self.requests_per_device_per_second = {} + # map (device, method) -> RateLimiter, populated on-demand + self.rate_limiters = {} # some config options are *only* read from filter conf at startup... default_conf_path = os.path.join( @@ -82,24 +76,35 @@ class BackendRateLimitMiddleware(object): self._load_config_file() def _refresh_ratelimiters(self): - for dev, rl in self.rate_limiters.items(): - rl.set_max_rate(self.requests_per_device_per_second) + # note: if we ever wanted to prune the ratelimiters (in case devices + # have been removed) we could inspect each ratelimiter's running_time + # and remove those with very old running_time + for (dev, method), rl in self.rate_limiters.items(): + rl.set_max_rate(self.requests_per_device_per_second[method]) rl.set_rate_buffer(self.requests_per_device_rate_buffer) def _apply_config(self, conf): - self.current_conf = conf modified = False - new_value = non_negative_float( - conf.get('requests_per_device_per_second', - DEFAULT_REQUESTS_PER_DEVICE_PER_SECOND)) - if new_value != self.requests_per_device_per_second: - self.requests_per_device_per_second = new_value - modified = True - new_value = non_negative_float( + reqs_per_device_rate_buffer = non_negative_float( conf.get('requests_per_device_rate_buffer', DEFAULT_REQUESTS_PER_DEVICE_RATE_BUFFER)) - if new_value != self.requests_per_device_rate_buffer: - self.requests_per_device_rate_buffer = new_value + + # note: 'None' key holds the aggregate per-device limit for all methods + reqs_per_device_per_second = {None: non_negative_float( + conf.get('requests_per_device_per_second', 0.0))} + for method in RATE_LIMITED_METHODS: + val = non_negative_float( + conf.get('%s_requests_per_device_per_second' + % method.lower(), 0.0)) + reqs_per_device_per_second[method] = val + + if reqs_per_device_rate_buffer != self.requests_per_device_rate_buffer: + self.requests_per_device_rate_buffer = reqs_per_device_rate_buffer + modified = True + if reqs_per_device_per_second != self.requests_per_device_per_second: + self.requests_per_device_per_second = reqs_per_device_per_second + self.is_any_rate_limit_configured = any( + self.requests_per_device_per_second.values()) modified = True if modified: self._refresh_ratelimiters() @@ -112,7 +117,7 @@ class BackendRateLimitMiddleware(object): # filter conf value or default value. If the conf file cannot be read # or is invalid, then the current config is left unchanged. try: - new_conf = dict(self.filter_conf) # filter_conf not current_conf + new_conf = dict(self.filter_conf) # filter_conf not current conf new_conf.update( readconf(self.conf_path, BACKEND_RATELIMIT_CONFIG_SECTION)) modified = self._apply_config(new_conf) @@ -150,6 +155,46 @@ class BackendRateLimitMiddleware(object): # always reset last loaded time to avoid re-try storm self._last_config_reload_attempt = now + def _get_ratelimiter(self, device, method=None): + """ + Get a rate limiter for the (device, method) combination. If a rate + limiter does not yet exist for the given (device, method) combination + then it is created and added to the map of rate limiters. + + :param: the device. + :method: the request method; if None then the aggregate rate limiter + for all requests to the device is returned. + :returns: an instance of ``EventletRateLimiter``. + """ + try: + rl = self.rate_limiters[(device, method)] + except KeyError: + rl = EventletRateLimiter( + max_rate=self.requests_per_device_per_second[method], + rate_buffer=self.requests_per_device_rate_buffer, + running_time=time.time(), + burst_after_idle=True) + self.rate_limiters[(device, method)] = rl + return rl + + def _is_allowed(self, device, method): + """ + Evaluate backend rate-limiting policies for the incoming request. + + A request is allowed when neither the per-(device, method) rate-limit + nor the per-device rate-limit has been reached. + + Note: a request will be disallowed if the aggregate per-device + rate-limit has been reached, even if the per-(device, method) + rate-limit has not been reached for the request's method. + + :param: the device. + :method: the request method. + :returns: boolean, is_allowed. + """ + return (self._get_ratelimiter(device, None).is_allowed() + and self._get_ratelimiter(device, method).is_allowed()) + def __call__(self, env, start_response): """ WSGI entry point. @@ -160,7 +205,7 @@ class BackendRateLimitMiddleware(object): self._maybe_reload_config() req = Request(env) handler = self.app - if (self.requests_per_device_per_second + if (self.is_any_rate_limit_configured and req.method in RATE_LIMITED_METHODS): try: device, partition, _ = split_and_validate_path(req, 1, 3, True) @@ -169,8 +214,7 @@ class BackendRateLimitMiddleware(object): # request may not have device/partition e.g. a healthcheck req pass else: - rate_limiter = self.rate_limiters[device] - if not rate_limiter.is_allowed(): + if not self._is_allowed(device, req.method): self.logger.increment('backend.ratelimit') handler = HTTPTooManyBackendRequests() return handler(env, start_response) diff --git a/test/unit/common/middleware/test_backend_ratelimit.py b/test/unit/common/middleware/test_backend_ratelimit.py index 68b106c31c..10b893aea9 100644 --- a/test/unit/common/middleware/test_backend_ratelimit.py +++ b/test/unit/common/middleware/test_backend_ratelimit.py @@ -46,23 +46,35 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): super(TestBackendRatelimitMiddleware, self).setUp() self.swift = FakeSwift() self.tempdir = mkdtemp() + self.default_req_per_dev_per_sec = dict( + (key, 0.0) for key in + (None, 'GET', 'HEAD', 'PUT', 'POST', 'DELETE', 'UPDATE', + 'REPLICATE') + ) def tearDown(self): shutil.rmtree(self.tempdir, ignore_errors=True) def test_init(self): - conf = {} + conf = {'swift_dir': self.tempdir} factory = backend_ratelimit.filter_factory(conf) rl = factory(self.swift) - self.assertEqual(0.0, rl.requests_per_device_per_second) + self.assertEqual(self.default_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(1.0, rl.requests_per_device_rate_buffer) + self.assertFalse(rl.is_any_rate_limit_configured) - conf = {'requests_per_device_per_second': 1.3, + conf = {'swift_dir': self.tempdir, + 'requests_per_device_per_second': 1.3, 'requests_per_device_rate_buffer': 2.4} factory = backend_ratelimit.filter_factory(conf) rl = factory(self.swift) - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + self.assertTrue(rl.is_any_rate_limit_configured) conf = {'requests_per_device_per_second': -1} factory = backend_ratelimit.filter_factory(conf) @@ -128,7 +140,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): 'swift.common.middleware.backend_ratelimit.get_logger', return_value=debug_logger()): rl = factory(self.swift) - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec.update({None: 1.3}) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(1.0, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('error')) self.assertEqual( @@ -141,13 +156,17 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): def test_init_config_file_unset_and_missing(self): # don't warn if missing conf file during init (conf_path not set) - conf = {'requests_per_device_per_second': "1.3"} + conf = {'swift_dir': self.tempdir, + 'requests_per_device_per_second': "1.3"} factory = backend_ratelimit.filter_factory(conf) with mock.patch( 'swift.common.middleware.backend_ratelimit.get_logger', return_value=debug_logger()): rl = factory(self.swift) - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(1.0, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('error')) self.assertEqual([], rl.logger.get_lines_for_level('warning')) @@ -164,7 +183,11 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): with mock.patch('swift.common.middleware.backend_ratelimit.get_logger', return_value=debug_logger()): rl = factory(self.swift) - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(1.0, rl.requests_per_device_rate_buffer) lines = rl.logger.get_lines_for_level('warning') self.assertEqual(1, len(lines), lines) self.assertIn('Invalid config file', lines[0]) @@ -186,9 +209,13 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # backend-ratelimit.conf overrides options - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) # but only the ones that are listed self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + self.assertTrue(rl.is_any_rate_limit_configured) lines = rl.logger.get_lines_for_level('info') self.assertEqual(['Loaded config file %s, config changed' % conf_path], lines) @@ -210,32 +237,151 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): with mock.patch('swift.common.middleware.backend_ratelimit.get_logger', return_value=debug_logger()): rl = factory(self.swift) - # we DO read rate limit options - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(12.4, rl.requests_per_device_rate_buffer) - # but we do NOT read config reload options + self.assertTrue(rl.is_any_rate_limit_configured) + # options related to conf file loading are not loaded from conf file... self.assertEqual(conf_path, rl.conf_path) self.assertEqual(15, rl.config_reload_interval) lines = rl.logger.logger.get_lines_for_level('info') self.assertEqual(['Loaded config file %s, config changed' % conf_path], lines) - def _do_test_config_file_reload(self, filter_conf, exp_reload_time): + def _do_test_init_config_file_overrides_filter_conf( + self, path_to_actual_conf_file, configured_conf_path): + # verify that conf file options override filter conf options + # create the actual file, but no options + with open(path_to_actual_conf_file, 'w') as fd: + fd.write('[backend_ratelimit]') + conf = {'swift_dir': self.tempdir, + 'requests_per_device_per_second': "1.3", + 'requests_per_device_rate_buffer': "2.4", + 'config_reload_interval': 15} + if configured_conf_path: + # only configure if given a conf_path + conf['backend_ratelimit_conf_path'] = configured_conf_path + exp_configured_conf_path = configured_conf_path + else: + # fall back to default + exp_configured_conf_path = os.path.join(self.tempdir, + 'backend-ratelimit.conf') + factory = backend_ratelimit.filter_factory(conf) + rl = factory(self.swift) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + self.assertTrue(rl.is_any_rate_limit_configured) + self.assertEqual(exp_configured_conf_path, rl.conf_path) + self.assertEqual(15, rl.config_reload_interval) + + # create file with option + with open(path_to_actual_conf_file, 'w') as fd: + fd.write('[backend_ratelimit]\n' + 'requests_per_device_per_second = 12.3\n' + 'backend_ratelimit_conf_path = /etc/swift/ignored.conf\n' + 'config_reload_interval = 999999\n') + factory = backend_ratelimit.filter_factory(conf) + rl = factory(self.swift) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + self.assertTrue(rl.is_any_rate_limit_configured) + # options related to conf file loading are not loaded from conf file... + self.assertEqual(exp_configured_conf_path, rl.conf_path) + self.assertEqual(15, rl.config_reload_interval) + + with open(path_to_actual_conf_file, 'w') as fd: + fd.write( + '[backend_ratelimit]\n' + 'requests_per_device_per_second = 5.3\n' + 'requests_per_device_rate_buffer = 0.5\n' + 'delete_requests_per_device_per_second = 1\n' + 'get_requests_per_device_per_second = 2\n' + 'head_requests_per_device_per_second = 3\n' + 'post_requests_per_device_per_second = 4\n' + 'put_requests_per_device_per_second = 5\n' + 'replicate_requests_per_device_per_second = 6\n' + 'update_requests_per_device_per_second = 7\n' + 'backend_ratelimit_conf_path = /etc/swift/ignored.conf\n' + 'config_reload_interval = 999999\n' + ) + factory = backend_ratelimit.filter_factory(conf) + rl = factory(self.swift) + exp_req_per_dev_per_sec.update( + { + None: 5.3, + 'DELETE': 1, + 'GET': 2, + 'HEAD': 3, + 'POST': 4, + 'PUT': 5, + 'REPLICATE': 6, + 'UPDATE': 7, + } + ) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(0.5, rl.requests_per_device_rate_buffer) + self.assertTrue(rl.is_any_rate_limit_configured) + # options related to conf file loading are not loaded from conf file... + self.assertEqual(exp_configured_conf_path, rl.conf_path) + self.assertEqual(15, rl.config_reload_interval) + + def test_init_config_file_at_default_path_overrides_filter_conf(self): + # default conf path is loaded if it exists + default_conf_path = os.path.join(self.tempdir, + 'backend-ratelimit.conf') + self._do_test_init_config_file_overrides_filter_conf( + path_to_actual_conf_file=default_conf_path, + configured_conf_path=None) + + self._do_test_init_config_file_overrides_filter_conf( + path_to_actual_conf_file=default_conf_path, + configured_conf_path=default_conf_path) + + def test_init_config_file_at_configured_path_overrides_filter_conf(self): + # explicitly configured conf path is loaded + custom_conf_path = os.path.join(self.tempdir, 'backend_rl.conf') + self._do_test_init_config_file_overrides_filter_conf( + path_to_actual_conf_file=custom_conf_path, + configured_conf_path=custom_conf_path) + + def _do_test_config_file_reload(self, reload_interval): # verify that conf file options are periodically reloaded + filter_conf = {'swift_dir': self.tempdir, + 'requests_per_device_per_second': "1.3", + 'requests_per_device_rate_buffer': "2.4", + 'head_requests_per_device_per_second': '6.2'} + if reload_interval: + filter_conf['config_reload_interval'] = reload_interval + now = time.time() # create the actual file - conf_path = os.path.join(self.tempdir, 'backend-ratelimit.conf') + conf_path = os.path.join(filter_conf['swift_dir'], + 'backend-ratelimit.conf') with open(conf_path, 'w') as fd: fd.write('[backend_ratelimit]\n' 'requests_per_device_per_second = 12.3\n' - 'backend_ratelimit_conf_path = /etc/swift/rl.conf\n' - 'config_reload_interval = 999999\n') + # conf file cannot re-configure where the conf file is... + 'backend_ratelimit_conf_path = /etc/ignored\n' + 'config_reload_interval = also_ignored\n') factory = backend_ratelimit.filter_factory(filter_conf) with mock.patch('swift.common.middleware.backend_ratelimit.time.time', return_value=now): rl = factory(self.swift) - self.assertEqual(12.3, rl.requests_per_device_per_second) - self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec.update({None: 12.3, 'HEAD': float( + filter_conf['head_requests_per_device_per_second'])}) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(float(filter_conf['requests_per_device_rate_buffer']), + rl.requests_per_device_rate_buffer) self.assertEqual(conf_path, rl.conf_path) # modify the conf file @@ -243,104 +389,198 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): fd.write('[backend_ratelimit]\n' 'requests_per_device_per_second = 29.3\n' 'requests_per_device_rate_buffer = 12.4\n' - 'backend_ratelimit_conf_path = /etc/swift/rl.conf\n' - 'config_reload_interval = 999999\n') + 'backend_ratelimit_conf_path = /etc/ignored\n' + 'config_reload_interval = also_ignored\n' + 'head_requests_per_device_per_second = 5.1\n' + 'delete_requests_per_device_per_second = 7.3\n' + 'get_requests_per_device_per_second = 8.4\n') # send some requests, but too soon for config file to be reloaded req1 = Request.blank('/sda1/99/a/c/o') - req2 = Request.blank('/sda2/99/a/c/o') + req2 = Request.blank('/sda2/99/a/c/o', + environ={'REQUEST_METHOD': 'DELETE'}) self.swift.register(req1.method, req1.path, HTTPOk, {}) self.swift.register(req2.method, req2.path, HTTPOk, {}) with mock.patch('swift.common.middleware.backend_ratelimit.time.time', - return_value=now + exp_reload_time - 1): + return_value=now + rl.config_reload_interval - 1): resp1 = req1.get_response(rl) resp2 = req2.get_response(rl) self.assertEqual(200, resp1.status_int) self.assertEqual(200, resp2.status_int) - self.assertEqual(12.3, rl.requests_per_device_per_second) - self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(float(filter_conf['requests_per_device_rate_buffer']), + rl.requests_per_device_rate_buffer) self.assertEqual(conf_path, rl.conf_path) + # verify the per dev ratelimiters + self.assertEqual({('sda1', 'GET'): 0.0, + ('sda2', 'DELETE'): 0.0, + ('sda1', None): 12.3, + ('sda2', None): 12.3}, + dict((key, val.max_rate) + for key, val in rl.rate_limiters.items())) + for (dev, method), limiter in rl.rate_limiters.items(): + self.assertEqual(2.4 * limiter.clock_accuracy, + limiter.rate_buffer_ms, (dev, method)) + # send some requests, time for config file to be reloaded with mock.patch('swift.common.middleware.backend_ratelimit.time.time', - return_value=now + exp_reload_time): + return_value=now + rl.config_reload_interval + 0.01): resp1 = req1.get_response(rl) resp2 = req2.get_response(rl) self.assertEqual(200, resp1.status_int) self.assertEqual(200, resp2.status_int) - self.assertEqual(29.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec.update({ + None: 29.3, + 'HEAD': 5.1, + 'DELETE': 7.3, + 'GET': 8.4}) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(12.4, rl.requests_per_device_rate_buffer) self.assertEqual(conf_path, rl.conf_path) # verify the per dev ratelimiters were updated - per_dev_rl_rates = [per_dev_rl.max_rate - for per_dev_rl in list(rl.rate_limiters.values())] - self.assertEqual([29.3, 29.3], per_dev_rl_rates) - per_dev_rl_buffer = [per_dev_rl.rate_buffer_ms - for per_dev_rl in list(rl.rate_limiters.values())] - self.assertEqual([12400, 12400], sorted(per_dev_rl_buffer)) + self.assertEqual({('sda1', 'GET'): 8.4, + ('sda2', 'DELETE'): 7.3, + ('sda1', None): 29.3, + ('sda2', None): 29.3}, + dict((key, val.max_rate) + for key, val in rl.rate_limiters.items())) + for (dev, method), limiter in rl.rate_limiters.items(): + self.assertEqual(12.4 * limiter.clock_accuracy, + limiter.rate_buffer_ms, (dev, method)) # modify the config file again # remove requests_per_device_per_second option + # remove [head|delete]_requests_per_device_per_second options with open(conf_path, 'w') as fd: fd.write('[backend_ratelimit]\n' - 'backend_ratelimit_conf_path = /etc/swift/rl.conf\n' - 'config_reload_interval = 999999\n') + 'backend_ratelimit_conf_path = /etc/ignored\n' + 'config_reload_interval = also_ignored\n' + 'requests_per_device_rate_buffer = 0.5\n' + 'get_requests_per_device_per_second = 9.5\n') # send some requests, not yet time for config file to be reloaded with mock.patch('swift.common.middleware.backend_ratelimit.time.time', - return_value=now + 2 * exp_reload_time - 1): + return_value=now + 2 * rl.config_reload_interval - 1): resp1 = req1.get_response(rl) resp2 = req2.get_response(rl) self.assertEqual(200, resp1.status_int) self.assertEqual(200, resp2.status_int) - self.assertEqual(29.3, rl.requests_per_device_per_second) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(12.4, rl.requests_per_device_rate_buffer) self.assertEqual(conf_path, rl.conf_path) # verify the per dev ratelimiters were not updated - per_dev_rl_rates = [per_dev_rl.max_rate - for per_dev_rl in list(rl.rate_limiters.values())] - self.assertEqual([29.3, 29.3], sorted(per_dev_rl_rates)) - per_dev_rl_buffer = [per_dev_rl.rate_buffer_ms - for per_dev_rl in list(rl.rate_limiters.values())] - self.assertEqual([12400, 12400], sorted(per_dev_rl_buffer)) + self.assertEqual({('sda1', 'GET'): 8.4, + ('sda2', 'DELETE'): 7.3, + ('sda1', None): 29.3, + ('sda2', None): 29.3}, + dict((key, val.max_rate) + for key, val in rl.rate_limiters.items())) + for (dev, method), limiter in rl.rate_limiters.items(): + self.assertEqual(12.4 * limiter.clock_accuracy, + limiter.rate_buffer_ms, (dev, method)) # send some requests, time for config file to be reloaded - with mock.patch('swift.common.middleware.backend_ratelimit.time.time', - return_value=now + 2 * exp_reload_time): + with mock.patch( + 'swift.common.middleware.backend_ratelimit.time.time', + return_value=now + 2 * rl.config_reload_interval + 0.01): resp1 = req1.get_response(rl) resp2 = req2.get_response(rl) self.assertEqual(200, resp1.status_int) self.assertEqual(200, resp2.status_int) # requests_per_device_per_second option reverts to filter conf - self.assertEqual(1.3, rl.requests_per_device_per_second) - self.assertEqual(2.4, rl.requests_per_device_rate_buffer) + # delete_requests_per_device_per_second option reverts to default + # head_requests_per_device_per_second option reverts to filter conf + exp_req_per_dev_per_sec.update({ + None: 1.3, + 'HEAD': 6.2, + 'DELETE': 0.0, + 'GET': 9.5}) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual(0.5, rl.requests_per_device_rate_buffer) self.assertEqual(conf_path, rl.conf_path) - # verify the per dev ratelimiters were not updated - per_dev_rl_rates = [per_dev_rl.max_rate - for per_dev_rl in list(rl.rate_limiters.values())] - self.assertEqual([1.3, 1.3], sorted(per_dev_rl_rates)) - per_dev_rl_buffer = [per_dev_rl.rate_buffer_ms - for per_dev_rl in list(rl.rate_limiters.values())] - self.assertEqual([2400, 2400], sorted(per_dev_rl_buffer)) + # verify the per dev ratelimiters were updated + self.assertEqual({('sda1', 'GET'): 9.5, + ('sda2', 'DELETE'): 0.0, + ('sda1', None): 1.3, + ('sda2', None): 1.3}, + dict((key, val.max_rate) + for key, val in rl.rate_limiters.items())) + for (dev, method), limiter in rl.rate_limiters.items(): + self.assertEqual(0.5 * limiter.clock_accuracy, + limiter.rate_buffer_ms, (dev, method)) return rl def test_config_file_reload_default_interval(self): - filter_conf = {'swift_dir': self.tempdir, - 'requests_per_device_per_second': "1.3", - 'requests_per_device_rate_buffer': "2.4"} - rl = self._do_test_config_file_reload(filter_conf, 60) + rl = self._do_test_config_file_reload(None) self.assertEqual(60, rl.config_reload_interval) def test_config_file_reload_custom_interval(self): + rl = self._do_test_config_file_reload(30.1) + self.assertEqual(30.1, rl.config_reload_interval) + + def test_config_file_reload_clears_all_limits(self): + # verify that reloaded config file can disable all rate limits + now = time.time() + conf_path = os.path.join(self.tempdir, 'missing') filter_conf = {'swift_dir': self.tempdir, - 'config_reload_interval': "30", + # path set so expect warning during init + 'backend_ratelimit_conf_path': conf_path, 'requests_per_device_per_second': "1.3", + 'head_requests_per_device_per_second = 1.1\n' 'requests_per_device_rate_buffer': "2.4"} - rl = self._do_test_config_file_reload(filter_conf, 30) - self.assertEqual(30, rl.config_reload_interval) + with open(conf_path, 'w') as fd: + fd.write('[backend_ratelimit]\n' + 'requests_per_device_per_second = 29.3\n' + 'head_requests_per_device_per_second = 5.1\n' + 'get_requests_per_device_per_second = 8.4\n') + factory = backend_ratelimit.filter_factory(filter_conf) + + # expect warning during init + with mock.patch('swift.common.middleware.backend_ratelimit.time.time', + return_value=now): + with mock.patch( + 'swift.common.middleware.backend_ratelimit.get_logger', + return_value=debug_logger()): + rl = factory(self.swift) + # filter conf has been applied + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec.update({ + None: 29.3, + 'HEAD': 5.1, + 'GET': 8.4}) + self.assertTrue(rl.is_any_rate_limit_configured) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual([], rl.logger.get_lines_for_level('warning')) + self.assertEqual([], rl.logger.get_lines_for_level('error')) + + # write zero rate limits to conf file + # jump into future, send request, config reload attempted + with open(conf_path, 'w') as fd: + fd.write('[backend_ratelimit]\n' + 'requests_per_device_per_second = 0.0\n' + 'head_requests_per_device_per_second = 0.0\n' + 'get_requests_per_device_per_second = 0.0\n') + req1 = Request.blank('/sda1/99/a/c/o') + self.swift.register(req1.method, req1.path, HTTPOk, {}) + with mock.patch('swift.common.middleware.backend_ratelimit.time.time', + return_value=now + 10000): + resp1 = req1.get_response(rl) + self.assertEqual(200, resp1.status_int) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + self.assertFalse(rl.is_any_rate_limit_configured) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) + self.assertEqual([], rl.logger.get_lines_for_level('warning')) + self.assertEqual([], rl.logger.get_lines_for_level('error')) def test_config_file_reload_set_and_missing(self): now = time.time() @@ -360,7 +600,11 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # filter conf has been applied - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertTrue(rl.is_any_rate_limit_configured) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual( ['Failed to load config file, config unchanged: Unable to read ' @@ -377,7 +621,9 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=now + 10000): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) - self.assertEqual(1.3, rl.requests_per_device_per_second) + self.assertTrue(rl.is_any_rate_limit_configured) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -398,7 +644,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # filter conf has been applied - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -412,7 +661,8 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) # previous conf file value has been retained - self.assertEqual(1.3, rl.requests_per_device_per_second) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -435,7 +685,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # conf file value has been applied - self.assertEqual(1.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 1.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -459,7 +712,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # conf file value has been applied - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -480,7 +736,8 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) # previous conf file value has been retained - self.assertEqual(12.3, rl.requests_per_device_per_second) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) mock_readconf.assert_called_once() self.assertEqual( @@ -495,7 +752,8 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) # previous conf file value has been retained - self.assertEqual(12.3, rl.requests_per_device_per_second) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -506,8 +764,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=now + 10060): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) - # updated conf file value is applied - self.assertEqual(29.3, rl.requests_per_device_per_second) + # previous conf file value has been retained + exp_req_per_dev_per_sec.update({None: 29.3}) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) self.assertEqual([], rl.logger.get_lines_for_level('warning')) self.assertEqual([], rl.logger.get_lines_for_level('error')) @@ -531,7 +791,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # conf file value has been applied - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) lines = rl.logger.get_lines_for_level('info') self.assertEqual(['Loaded config file %s, config changed' % conf_path], @@ -545,7 +808,8 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=now + 10000): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) - self.assertEqual(12.3, rl.requests_per_device_per_second) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) lines = rl.logger.get_lines_for_level('info') self.assertEqual([], lines) @@ -560,7 +824,9 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): resp1 = req1.get_response(rl) self.assertEqual(200, resp1.status_int) # previous conf file value has been retained - self.assertEqual(23.4, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec.update({None: 23.4}) + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) lines = rl.logger.get_lines_for_level('info') self.assertEqual(['Loaded config file %s, config changed' % conf_path], @@ -586,7 +852,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): return_value=debug_logger()): rl = factory(self.swift) # conf file value has been applied - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) lines = rl.logger.get_lines_for_level('info') self.assertEqual( @@ -656,7 +925,10 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): with mock.patch('swift.common.middleware.backend_ratelimit.time.time', return_value=now): rl = factory(self.swift) - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) with open(conf_path, 'w') as fd: @@ -671,10 +943,14 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): resp = req.get_response(rl) self.assertEqual(200, resp.status_int) # no change - self.assertEqual(12.3, rl.requests_per_device_per_second) + exp_req_per_dev_per_sec = dict(self.default_req_per_dev_per_sec) + exp_req_per_dev_per_sec[None] = 12.3 + self.assertEqual(exp_req_per_dev_per_sec, + rl.requests_per_device_per_second) self.assertEqual(2.4, rl.requests_per_device_rate_buffer) - def _do_test_ratelimit(self, method, req_per_sec, rate_buffer): + def _do_test_ratelimit(self, method, req_per_sec, rate_buffer, + extra_conf=None): # send 20 requests, time increments by 0.01 between each request start = time.time() fake_time = [start] @@ -685,8 +961,11 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): app = FakeSwift() logger = debug_logger() # apply a ratelimit - conf = {'requests_per_device_per_second': req_per_sec, + conf = {'swift_dir': self.tempdir, + 'requests_per_device_per_second': req_per_sec, 'requests_per_device_rate_buffer': rate_buffer} + if extra_conf: + conf.update(extra_conf) rl = BackendRateLimitMiddleware(app, conf, logger) success = defaultdict(int) ratelimited = 0 @@ -712,13 +991,13 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): 'backend.ratelimit', 0)) return success - def test_ratelimited(self): + def test_method_ratelimited(self): def do_test_ratelimit(method): # no rate-limiting success_per_dev = self._do_test_ratelimit(method, 0, 0) self.assertEqual([20] * 3, list(success_per_dev.values())) - # rate-limited + # global rate-limited success_per_dev = self._do_test_ratelimit(method, 1, 0) self.assertEqual([1] * 3, list(success_per_dev.values())) @@ -734,6 +1013,20 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): success_per_dev = self._do_test_ratelimit(method, 10, 1) self.assertEqual([12] * 3, list(success_per_dev.values())) + # method rate-limited + extra_conf = { + '%s_requests_per_device_per_second' % method.lower(): 1 + } + success_per_dev = self._do_test_ratelimit(method, 0, 0, extra_conf) + self.assertEqual([1] * 3, list(success_per_dev.values())) + + # method not rate-limited, global rate limited + extra_conf = { + '%s_requests_per_device_per_second' % method.lower(): 100 + } + success_per_dev = self._do_test_ratelimit(method, 1, 0, extra_conf) + self.assertEqual([1] * 3, list(success_per_dev.values())) + do_test_ratelimit('GET') do_test_ratelimit('HEAD') do_test_ratelimit('PUT') @@ -742,7 +1035,7 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): do_test_ratelimit('UPDATE') do_test_ratelimit('REPLICATE') - def test_not_ratelimited(self): + def test_method_not_ratelimited(self): def do_test_no_ratelimit(method): # verify no rate-limiting success_per_dev = self._do_test_ratelimit(method, 1, 0) @@ -751,6 +1044,15 @@ class TestBackendRatelimitMiddleware(unittest.TestCase): do_test_no_ratelimit('OPTIONS') do_test_no_ratelimit('SSYNC') + def test_no_ratelimiting_configured(self): + # verify shortcut path when no ratelimiting is configured + with mock.patch( + 'swift.common.middleware.backend_ratelimit.' + 'BackendRateLimitMiddleware._is_allowed') as mock_is_allowed: + success_per_dev = self._do_test_ratelimit('GET', 0, 0) + self.assertEqual([20] * 3, list(success_per_dev.values())) + mock_is_allowed.assert_not_called() + def test_unhandled_request(self): app = FakeSwift() logger = debug_logger()