From 6e313e957d2fd7e02bc3abe59f6a3c6cd37f23a2 Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Tue, 19 Nov 2013 22:55:09 -0500 Subject: [PATCH] Fix for memcache middleware configuration The documentation rightly said to use "memcache_max_connections", but the code was looking for "max_connections", and only looking for it in proxy-server.conf, not in memcache.conf as a fall back. This commit brings the code coverage for the memcache middleware to 100%. Closes-Bug: 1252893 Change-Id: I6ea64baa2f961a09d60b977b40d5baf842449ece Signed-off-by: Peter Portante --- etc/memcache.conf-sample | 3 + etc/proxy-server.conf-sample | 3 + swift/common/middleware/memcache.py | 22 +- test/unit/common/middleware/test_memcache.py | 207 ++++++++++++++++--- 4 files changed, 210 insertions(+), 25 deletions(-) diff --git a/etc/memcache.conf-sample b/etc/memcache.conf-sample index 5ad48ab100..24d24c8103 100644 --- a/etc/memcache.conf-sample +++ b/etc/memcache.conf-sample @@ -13,3 +13,6 @@ # set to 2 and reload. # In the future, the ability to use pickle serialization will be removed. # memcache_serialization_support = 2 +# +# Sets the maximum number of connections to each memcached server per worker +# memcache_max_connections = 2 diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 975bf4902a..4872c479d7 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -298,6 +298,9 @@ use = egg:swift#memcache # set to 2 and reload. # In the future, the ability to use pickle serialization will be removed. # memcache_serialization_support = 2 +# +# Sets the maximum number of connections to each memcached server per worker +# memcache_max_connections = 2 [filter:ratelimit] use = egg:swift#ratelimit diff --git a/swift/common/middleware/memcache.py b/swift/common/middleware/memcache.py index 3252c7f8fe..19a2f4197f 100644 --- a/swift/common/middleware/memcache.py +++ b/swift/common/middleware/memcache.py @@ -28,9 +28,17 @@ class MemcacheMiddleware(object): self.app = app self.memcache_servers = conf.get('memcache_servers') serialization_format = conf.get('memcache_serialization_support') - max_conns = int(conf.get('max_connections', 2)) + try: + # Originally, while we documented using memcache_max_connections + # we only accepted max_connections + max_conns = int(conf.get('memcache_max_connections', + conf.get('max_connections', 0))) + except ValueError: + max_conns = 0 - if not self.memcache_servers or serialization_format is None: + if (not self.memcache_servers + or serialization_format is None + or max_conns <= 0): path = os.path.join(conf.get('swift_dir', '/etc/swift'), 'memcache.conf') memcache_conf = ConfigParser() @@ -48,9 +56,19 @@ class MemcacheMiddleware(object): 'memcache_serialization_support') except (NoSectionError, NoOptionError): pass + if max_conns <= 0: + try: + new_max_conns = \ + memcache_conf.get('memcache', + 'memcache_max_connections') + max_conns = int(new_max_conns) + except (NoSectionError, NoOptionError, ValueError): + pass if not self.memcache_servers: self.memcache_servers = '127.0.0.1:11211' + if max_conns <= 0: + max_conns = 2 if serialization_format is None: serialization_format = 2 else: diff --git a/test/unit/common/middleware/test_memcache.py b/test/unit/common/middleware/test_memcache.py index 8d3d7781dd..3273ee5a65 100644 --- a/test/unit/common/middleware/test_memcache.py +++ b/test/unit/common/middleware/test_memcache.py @@ -38,21 +38,41 @@ class EmptyConfigParser(object): return False -class SetConfigParser(object): +def get_config_parser(memcache_servers='1.2.3.4:5', + memcache_serialization_support='1', + memcache_max_connections='4', + section='memcache'): + _srvs = memcache_servers + _sers = memcache_serialization_support + _maxc = memcache_max_connections + _section = section - def read(self, path): - return True + class SetConfigParser(object): - def get(self, section, option): - if section == 'memcache': - if option == 'memcache_servers': - return '1.2.3.4:5' - elif option == 'memcache_serialization_support': - return '1' + def read(self, path): + return True + + def get(self, section, option): + if _section == section: + if option == 'memcache_servers': + if _srvs == 'error': + raise NoOptionError(option, section) + return _srvs + elif option == 'memcache_serialization_support': + if _sers == 'error': + raise NoOptionError(option, section) + return _sers + elif option in ('memcache_max_connections', + 'max_connections'): + if _maxc == 'error': + raise NoOptionError(option, section) + return _maxc + else: + raise NoOptionError(option, section) else: - raise NoOptionError(option) - else: - raise NoSectionError(option) + raise NoSectionError(option) + + return SetConfigParser def start_response(*args): @@ -73,15 +93,29 @@ class TestCacheMiddleware(unittest.TestCase): def test_conf_default_read(self): orig_parser = memcache.ConfigParser memcache.ConfigParser = ExcConfigParser - exc = None + count = 0 try: - memcache.MemcacheMiddleware(FakeApp(), {}) - except Exception as err: - exc = err + for d in ({}, + {'memcache_servers': '6.7.8.9:10'}, + {'memcache_serialization_support': '0'}, + {'memcache_max_connections': '30'}, + {'memcache_servers': '6.7.8.9:10', + 'memcache_serialization_support': '0'}, + {'memcache_servers': '6.7.8.9:10', + 'memcache_max_connections': '30'}, + {'memcache_serialization_support': '0', + 'memcache_max_connections': '30'} + ): + try: + memcache.MemcacheMiddleware(FakeApp(), d) + except Exception as err: + self.assertEquals( + str(err), + "read called with '/etc/swift/memcache.conf'") + count += 1 finally: memcache.ConfigParser = orig_parser - self.assertEquals(str(exc), - "read called with '/etc/swift/memcache.conf'") + self.assertEquals(count, 7) def test_conf_set_no_read(self): orig_parser = memcache.ConfigParser @@ -90,7 +124,8 @@ class TestCacheMiddleware(unittest.TestCase): try: memcache.MemcacheMiddleware( FakeApp(), {'memcache_servers': '1.2.3.4:5', - 'memcache_serialization_support': '2'}) + 'memcache_serialization_support': '2', + 'memcache_max_connections': '30'}) except Exception as err: exc = err finally: @@ -107,10 +142,91 @@ class TestCacheMiddleware(unittest.TestCase): self.assertEquals(app.memcache_servers, '127.0.0.1:11211') self.assertEquals(app.memcache._allow_pickle, False) self.assertEquals(app.memcache._allow_unpickle, False) + self.assertEquals( + app.memcache._client_cache['127.0.0.1:11211'].max_size, 2) + + def test_conf_inline(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser() + try: + app = memcache.MemcacheMiddleware( + FakeApp(), + {'memcache_servers': '6.7.8.9:10', + 'memcache_serialization_support': '0', + 'memcache_max_connections': '5'}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '6.7.8.9:10') + self.assertEquals(app.memcache._allow_pickle, True) + self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['6.7.8.9:10'].max_size, 5) + + def test_conf_extra_no_section(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser(section='foobar') + try: + app = memcache.MemcacheMiddleware(FakeApp(), {}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '127.0.0.1:11211') + self.assertEquals(app.memcache._allow_pickle, False) + self.assertEquals(app.memcache._allow_unpickle, False) + self.assertEquals( + app.memcache._client_cache['127.0.0.1:11211'].max_size, 2) + + def test_conf_extra_no_option(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser( + memcache_servers='error', memcache_serialization_support='error', + memcache_max_connections='error') + try: + app = memcache.MemcacheMiddleware(FakeApp(), {}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '127.0.0.1:11211') + self.assertEquals(app.memcache._allow_pickle, False) + self.assertEquals(app.memcache._allow_unpickle, False) + self.assertEquals( + app.memcache._client_cache['127.0.0.1:11211'].max_size, 2) + + def test_conf_inline_other_max_conn(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser() + try: + app = memcache.MemcacheMiddleware( + FakeApp(), + {'memcache_servers': '6.7.8.9:10', + 'memcache_serialization_support': '0', + 'max_connections': '5'}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '6.7.8.9:10') + self.assertEquals(app.memcache._allow_pickle, True) + self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['6.7.8.9:10'].max_size, 5) + + def test_conf_inline_bad_max_conn(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser() + try: + app = memcache.MemcacheMiddleware( + FakeApp(), + {'memcache_servers': '6.7.8.9:10', + 'memcache_serialization_support': '0', + 'max_connections': 'bad42'}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '6.7.8.9:10') + self.assertEquals(app.memcache._allow_pickle, True) + self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['6.7.8.9:10'].max_size, 4) def test_conf_from_extra_conf(self): orig_parser = memcache.ConfigParser - memcache.ConfigParser = SetConfigParser + memcache.ConfigParser = get_config_parser() try: app = memcache.MemcacheMiddleware(FakeApp(), {}) finally: @@ -118,21 +234,66 @@ class TestCacheMiddleware(unittest.TestCase): self.assertEquals(app.memcache_servers, '1.2.3.4:5') self.assertEquals(app.memcache._allow_pickle, False) self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['1.2.3.4:5'].max_size, 4) - def test_conf_from_inline_conf(self): + def test_conf_from_extra_conf_bad_max_conn(self): orig_parser = memcache.ConfigParser - memcache.ConfigParser = SetConfigParser + memcache.ConfigParser = get_config_parser( + memcache_max_connections='bad42') + try: + app = memcache.MemcacheMiddleware(FakeApp(), {}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '1.2.3.4:5') + self.assertEquals(app.memcache._allow_pickle, False) + self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['1.2.3.4:5'].max_size, 2) + + def test_conf_from_inline_and_maxc_from_extra_conf(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser() try: app = memcache.MemcacheMiddleware( FakeApp(), {'memcache_servers': '6.7.8.9:10', - 'serialization_format': '0'}) + 'memcache_serialization_support': '0'}) + finally: + memcache.ConfigParser = orig_parser + self.assertEquals(app.memcache_servers, '6.7.8.9:10') + self.assertEquals(app.memcache._allow_pickle, True) + self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['6.7.8.9:10'].max_size, 4) + + def test_conf_from_inline_and_sers_from_extra_conf(self): + orig_parser = memcache.ConfigParser + memcache.ConfigParser = get_config_parser() + try: + app = memcache.MemcacheMiddleware( + FakeApp(), + {'memcache_servers': '6.7.8.9:10', + 'memcache_max_connections': '42'}) finally: memcache.ConfigParser = orig_parser self.assertEquals(app.memcache_servers, '6.7.8.9:10') self.assertEquals(app.memcache._allow_pickle, False) self.assertEquals(app.memcache._allow_unpickle, True) + self.assertEquals( + app.memcache._client_cache['6.7.8.9:10'].max_size, 42) + def test_filter_factory(self): + factory = memcache.filter_factory({'max_connections': '3'}, + memcache_servers='10.10.10.10:10', + memcache_serialization_support='1') + thefilter = factory('myapp') + self.assertEquals(thefilter.app, 'myapp') + self.assertEquals(thefilter.memcache_servers, '10.10.10.10:10') + self.assertEquals(thefilter.memcache._allow_pickle, False) + self.assertEquals(thefilter.memcache._allow_unpickle, True) + self.assertEquals( + thefilter.memcache._client_cache['10.10.10.10:10'].max_size, 3) if __name__ == '__main__': unittest.main()