diff --git a/swift/common/middleware/ratelimit.py b/swift/common/middleware/ratelimit.py index dad907c6d3..5eddd66dec 100644 --- a/swift/common/middleware/ratelimit.py +++ b/swift/common/middleware/ratelimit.py @@ -118,7 +118,8 @@ class RateLimitMiddleware(object): container_name) container_info = self.memcache_client.get(memcache_key) if isinstance(container_info, dict): - container_size = container_info.get('container_size', 0) + container_size = container_info.get( + 'count', container_info.get('container_size', 0)) container_rate = self.get_container_maxrate(container_size) if container_rate: keys.append(("ratelimit/%s/%s" % (account_name, diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index e68d32626f..c2e03c8212 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -266,89 +266,79 @@ class Controller(object): :param account: account name for the container :param container: container name to look up - :returns: tuple of (container partition, container nodes, container - read acl, container write acl, container sync key) or (None, - None, None, None, None) if the container does not exist + :returns: dict containing at least container partition ('partition'), + container nodes ('containers'), container read + acl ('read_acl'), container write acl ('write_acl'), + and container sync key ('sync_key'). + Values are set to None if the container does not exist. """ - partition, nodes = self.app.container_ring.get_nodes( - account, container) + part, nodes = self.app.container_ring.get_nodes(account, container) path = '/%s/%s' % (account, container) + container_info = {'status': 0, 'read_acl': None, + 'write_acl': None, 'sync_key': None, + 'count': None, 'bytes': None, + 'versions': None, 'partition': None, + 'nodes': None} if self.app.memcache: cache_key = get_container_memcache_key(account, container) cache_value = self.app.memcache.get(cache_key) if isinstance(cache_value, dict): - status = cache_value['status'] - read_acl = cache_value['read_acl'] - write_acl = cache_value['write_acl'] - sync_key = cache_value.get('sync_key') - versions = cache_value.get('versions') - if status == HTTP_OK: - return partition, nodes, read_acl, write_acl, sync_key, \ - versions - elif status == HTTP_NOT_FOUND: - return None, None, None, None, None, None + if 'container_size' in cache_value: + cache_value['count'] = cache_value['container_size'] + if is_success(cache_value['status']): + container_info.update(cache_value) + container_info['partition'] = part + container_info['nodes'] = nodes + return container_info if not self.account_info(account, autocreate=account_autocreate)[1]: - return None, None, None, None, None, None - result_code = 0 - read_acl = None - write_acl = None - sync_key = None - container_size = None - versions = None + return container_info attempts_left = len(nodes) headers = {'x-trans-id': self.trans_id, 'Connection': 'close'} - iternodes = self.iter_nodes(partition, nodes, self.app.container_ring) - while attempts_left > 0: - try: - node = iternodes.next() - except StopIteration: - break - attempts_left -= 1 + for node in self.iter_nodes(part, nodes, self.app.container_ring): try: with ConnectionTimeout(self.app.conn_timeout): conn = http_connect(node['ip'], node['port'], - node['device'], partition, 'HEAD', path, headers) + node['device'], part, 'HEAD', + path, headers) with Timeout(self.app.node_timeout): resp = conn.getresponse() body = resp.read() - if is_success(resp.status): - result_code = HTTP_OK - read_acl = resp.getheader('x-container-read') - write_acl = resp.getheader('x-container-write') - sync_key = resp.getheader('x-container-sync-key') - container_size = \ - resp.getheader('X-Container-Object-Count') - versions = resp.getheader('x-versions-location') - break - elif resp.status == HTTP_NOT_FOUND: - if result_code == 0: - result_code = HTTP_NOT_FOUND - elif result_code != HTTP_NOT_FOUND: - result_code = -1 - elif resp.status == HTTP_INSUFFICIENT_STORAGE: + if is_success(resp.status): + container_info.update({ + 'status': HTTP_OK, + 'read_acl': resp.getheader('x-container-read'), + 'write_acl': resp.getheader('x-container-write'), + 'sync_key': resp.getheader('x-container-sync-key'), + 'count': resp.getheader('x-container-object-count'), + 'bytes': resp.getheader('x-container-bytes-used'), + 'versions': resp.getheader('x-versions-location')}) + break + elif resp.status == HTTP_NOT_FOUND: + container_info['status'] = HTTP_NOT_FOUND + else: + container_info['status'] = -1 + if resp.status == HTTP_INSUFFICIENT_STORAGE: self.error_limit(node) - continue - else: - result_code = -1 except (Exception, Timeout): - self.exception_occurred(node, _('Container'), + self.exception_occurred( + node, _('Container'), _('Trying to get container info for %s') % path) - if self.app.memcache and result_code in (HTTP_OK, HTTP_NOT_FOUND): - if result_code == HTTP_OK: - cache_timeout = self.app.recheck_container_existence - else: - cache_timeout = self.app.recheck_container_existence * 0.1 - self.app.memcache.set(cache_key, - {'status': result_code, - 'read_acl': read_acl, - 'write_acl': write_acl, - 'sync_key': sync_key, - 'container_size': container_size, - 'versions': versions}, - timeout=cache_timeout) - if result_code == HTTP_OK: - return partition, nodes, read_acl, write_acl, sync_key, versions - return None, None, None, None, None, None + attempts_left -= 1 + if attempts_left <= 0: + break + if self.app.memcache: + if container_info['status'] == HTTP_OK: + self.app.memcache.set( + cache_key, container_info, + timeout=self.app.recheck_container_existence) + elif container_info['status'] == HTTP_NOT_FOUND: + self.app.memcache.set( + cache_key, container_info, + timeout=self.app.recheck_container_existence * 0.1) + if container_info['status'] == HTTP_OK: + container_info['partition'] = part + container_info['nodes'] = nodes + return container_info def iter_nodes(self, partition, nodes, ring): """ diff --git a/swift/proxy/controllers/container.py b/swift/proxy/controllers/container.py index c3616a079e..1b349c99d3 100644 --- a/swift/proxy/controllers/container.py +++ b/swift/proxy/controllers/container.py @@ -82,7 +82,8 @@ class ContainerController(Controller): 'read_acl': resp.headers.get('x-container-read'), 'write_acl': resp.headers.get('x-container-write'), 'sync_key': resp.headers.get('x-container-sync-key'), - 'container_size': resp.headers.get('x-container-object-count'), + 'count': resp.headers.get('x-container-object-count'), + 'bytes': resp.headers.get('x-container-bytes-used'), 'versions': resp.headers.get('x-versions-location')}, timeout=self.app.recheck_container_existence) @@ -173,9 +174,8 @@ class ContainerController(Controller): 'Connection': 'close'} self.transfer_headers(req.headers, headers) if self.app.memcache: - cache_key = get_container_memcache_key(self.account_name, - self.container_name) - self.app.memcache.delete(cache_key) + self.app.memcache.delete(get_container_memcache_key( + self.account_name, self.container_name)) resp = self.make_requests(req, self.app.container_ring, container_partition, 'POST', req.path_info, [headers] * len(containers)) diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index fcb2c7493a..e09020ad0a 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -278,8 +278,9 @@ class ObjectController(Controller): def GETorHEAD(self, req): """Handle HTTP GET or HEAD requests.""" - _junk, _junk, req.acl, _junk, _junk, object_versions = \ - self.container_info(self.account_name, self.container_name) + container_info = self.container_info(self.account_name, + self.container_name) + req.acl = container_info['read_acl'] if 'swift.authorize' in req.environ: aresp = req.environ['swift.authorize'](req) if aresp: @@ -413,9 +414,12 @@ class ObjectController(Controller): error_response = check_metadata(req, 'object') if error_response: return error_response - container_partition, containers, _junk, req.acl, _junk, _junk = \ - self.container_info(self.account_name, self.container_name, - account_autocreate=self.app.account_autocreate) + container_info = self.container_info( + self.account_name, self.container_name, + account_autocreate=self.app.account_autocreate) + container_partition = container_info['partition'] + containers = container_info['nodes'] + req.acl = container_info['write_acl'] if 'swift.authorize' in req.environ: aresp = req.environ['swift.authorize'](req) if aresp: @@ -498,10 +502,14 @@ class ObjectController(Controller): @delay_denial def PUT(self, req): """HTTP PUT request handler.""" - (container_partition, containers, _junk, req.acl, - req.environ['swift_sync_key'], object_versions) = \ - self.container_info(self.account_name, self.container_name, - account_autocreate=self.app.account_autocreate) + container_info = self.container_info( + self.account_name, self.container_name, + account_autocreate=self.app.account_autocreate) + container_partition = container_info['partition'] + containers = container_info['nodes'] + req.acl = container_info['write_acl'] + req.environ['swift_sync_key'] = container_info['sync_key'] + object_versions = container_info['versions'] if 'swift.authorize' in req.environ: aresp = req.environ['swift.authorize'](req) if aresp: @@ -776,9 +784,13 @@ class ObjectController(Controller): @delay_denial def DELETE(self, req): """HTTP DELETE request handler.""" - (container_partition, containers, _junk, req.acl, - req.environ['swift_sync_key'], object_versions) = \ - self.container_info(self.account_name, self.container_name) + container_info = self.container_info(self.account_name, + self.container_name) + container_partition = container_info['partition'] + containers = container_info['nodes'] + req.acl = container_info['write_acl'] + req.environ['swift_sync_key'] = container_info['sync_key'] + object_versions = container_info['versions'] if object_versions: # this is a version manifest and needs to be handled differently lcontainer = object_versions.split('/')[0] @@ -824,9 +836,11 @@ class ObjectController(Controller): self.container_name = lcontainer self.object_name = last_item['name'] new_del_req = Request.blank(copy_path, environ=req.environ) - (container_partition, containers, - _junk, new_del_req.acl, _junk, _junk) = \ - self.container_info(self.account_name, self.container_name) + container_info = self.container_info(self.account_name, + self.container_name) + container_partition = container_info['partition'] + containers = container_info['nodes'] + new_del_req.acl = container_info['write_acl'] new_del_req.path_info = copy_path req = new_del_req if 'swift.authorize' in req.environ: diff --git a/test/unit/common/middleware/test_ratelimit.py b/test/unit/common/middleware/test_ratelimit.py index 3db4e01bd1..38b7cd09c6 100644 --- a/test/unit/common/middleware/test_ratelimit.py +++ b/test/unit/common/middleware/test_ratelimit.py @@ -184,7 +184,7 @@ class TestRateLimit(unittest.TestCase): 'container_ratelimit_3': 200} fake_memcache = FakeMemcache() fake_memcache.store[get_container_memcache_key('a', 'c')] = \ - {'container_size': 5} + {'count': 5} the_app = ratelimit.RateLimitMiddleware(None, conf_dict, logger=FakeLogger()) the_app.memcache_client = fake_memcache @@ -199,6 +199,19 @@ class TestRateLimit(unittest.TestCase): self.assertEquals(len(the_app.get_ratelimitable_key_tuples( 'PUT', 'a', 'c', 'o')), 1) + def test_ratelimit_old_memcache_format(self): + current_rate = 13 + conf_dict = {'account_ratelimit': current_rate, + 'container_ratelimit_3': 200} + fake_memcache = FakeMemcache() + fake_memcache.store[get_container_memcache_key('a', 'c')] = \ + {'container_size': 5} + the_app = ratelimit.RateLimitMiddleware(None, conf_dict, + logger=FakeLogger()) + the_app.memcache_client = fake_memcache + tuples = the_app.get_ratelimitable_key_tuples('PUT', 'a', 'c', 'o') + self.assertEquals(tuples, [('ratelimit/a/c', 200.0)]) + def test_account_ratelimit(self): current_rate = 5 num_calls = 50 diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index db64d02699..149e88eb57 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -502,10 +502,10 @@ class TestController(unittest.TestCase): partition, nodes = self.container_ring.get_nodes(self.account, self.container) read_acl, write_acl = self.read_acl, self.write_acl - self.assertEqual(partition, ret[0]) - self.assertEqual(nodes, ret[1]) - self.assertEqual(read_acl, ret[2]) - self.assertEqual(write_acl, ret[3]) + self.assertEqual(partition, ret['partition']) + self.assertEqual(nodes, ret['nodes']) + self.assertEqual(read_acl, ret['read_acl']) + self.assertEqual(write_acl, ret['write_acl']) def test_container_info_invalid_account(self): def account_info(self, account, autocreate=False):