diff --git a/etc/memcache.conf-sample b/etc/memcache.conf-sample index 18faa668ee..b375eb4029 100644 --- a/etc/memcache.conf-sample +++ b/etc/memcache.conf-sample @@ -56,3 +56,9 @@ # specified in tls_certfile. If tls_enabled is False, this option # is ignored. # tls_keyfile = +# +# If an item size ever gets above item_size_warning_threshold then a warning will be +# logged. This can be used to alert when memcache item sizes are getting to their limit. +# It's an absolute size in bytes. Setting the value to 0 will warn on every memcache set. +# A value of -1 disables the warning. +# item_size_warning_threshold = -1 diff --git a/swift/common/memcached.py b/swift/common/memcached.py index 0493f747f8..1a371e58b6 100644 --- a/swift/common/memcached.py +++ b/swift/common/memcached.py @@ -56,7 +56,7 @@ from eventlet.pools import Pool from eventlet import Timeout from six.moves import range from swift.common import utils -from swift.common.utils import md5 +from swift.common.utils import md5, human_readable DEFAULT_MEMCACHED_PORT = 11211 @@ -73,6 +73,7 @@ TRY_COUNT = 3 # will be considered failed for ERROR_LIMIT_DURATION seconds. ERROR_LIMIT_COUNT = 10 ERROR_LIMIT_TIME = ERROR_LIMIT_DURATION = 60 +DEFAULT_ITEM_SIZE_WARNING_THRESHOLD = -1 def md5hash(key): @@ -172,13 +173,15 @@ class MemcacheRing(object): Simple, consistent-hashed memcache client. """ - def __init__(self, servers, connect_timeout=CONN_TIMEOUT, - io_timeout=IO_TIMEOUT, pool_timeout=POOL_TIMEOUT, - tries=TRY_COUNT, allow_pickle=False, allow_unpickle=False, - max_conns=2, tls_context=None, logger=None, - error_limit_count=ERROR_LIMIT_COUNT, - error_limit_time=ERROR_LIMIT_TIME, - error_limit_duration=ERROR_LIMIT_DURATION): + def __init__( + self, servers, connect_timeout=CONN_TIMEOUT, + io_timeout=IO_TIMEOUT, pool_timeout=POOL_TIMEOUT, + tries=TRY_COUNT, allow_pickle=False, allow_unpickle=False, + max_conns=2, tls_context=None, logger=None, + error_limit_count=ERROR_LIMIT_COUNT, + error_limit_time=ERROR_LIMIT_TIME, + error_limit_duration=ERROR_LIMIT_DURATION, + item_size_warning_threshold=DEFAULT_ITEM_SIZE_WARNING_THRESHOLD): self._ring = {} self._errors = dict(((serv, []) for serv in servers)) self._error_limited = dict(((serv, 0) for serv in servers)) @@ -203,6 +206,7 @@ class MemcacheRing(object): self.logger = logging.getLogger() else: self.logger = logger + self.item_size_warning_threshold = item_size_warning_threshold def _exception_occurred(self, server, e, action='talking', sock=None, fp=None, got_connection=True): @@ -325,6 +329,13 @@ class MemcacheRing(object): "Error setting value in memcached: " "%(server)s: %(msg)s", {'server': server, 'msg': msg}) + if 0 <= self.item_size_warning_threshold <= len(value): + self.logger.warning( + "Item size larger than warning threshold: " + "%d (%s) >= %d (%s)", len(value), + human_readable(len(value)), + self.item_size_warning_threshold, + human_readable(self.item_size_warning_threshold)) self._return_conn(server, fp, sock) return except (Exception, Timeout) as e: diff --git a/swift/common/middleware/memcache.py b/swift/common/middleware/memcache.py index c20824aed4..4fa2b551ca 100644 --- a/swift/common/middleware/memcache.py +++ b/swift/common/middleware/memcache.py @@ -20,7 +20,7 @@ from six.moves.configparser import ConfigParser, NoSectionError, NoOptionError from swift.common.memcached import ( MemcacheRing, CONN_TIMEOUT, POOL_TIMEOUT, IO_TIMEOUT, TRY_COUNT, - ERROR_LIMIT_COUNT, ERROR_LIMIT_TIME) + ERROR_LIMIT_COUNT, ERROR_LIMIT_TIME, DEFAULT_ITEM_SIZE_WARNING_THRESHOLD) from swift.common.utils import get_logger, config_true_value @@ -103,6 +103,9 @@ class MemcacheMiddleware(object): 'error_suppression_interval', ERROR_LIMIT_TIME)) error_suppression_limit = float(memcache_options.get( 'error_suppression_limit', ERROR_LIMIT_COUNT)) + item_size_warning_threshold = int(memcache_options.get( + 'item_size_warning_threshold', + DEFAULT_ITEM_SIZE_WARNING_THRESHOLD)) if not self.memcache_servers: self.memcache_servers = '127.0.0.1:11211' @@ -126,7 +129,8 @@ class MemcacheMiddleware(object): logger=self.logger, error_limit_count=error_suppression_limit, error_limit_time=error_suppression_interval, - error_limit_duration=error_suppression_interval) + error_limit_duration=error_suppression_interval, + item_size_warning_threshold=item_size_warning_threshold) def __call__(self, env, start_response): env['swift.cache'] = self.memcache diff --git a/test/unit/common/middleware/test_memcache.py b/test/unit/common/middleware/test_memcache.py index 5d2bc811e3..a5fe65fe63 100644 --- a/test/unit/common/middleware/test_memcache.py +++ b/test/unit/common/middleware/test_memcache.py @@ -49,11 +49,13 @@ class EmptyConfigParser(object): def get_config_parser(memcache_servers='1.2.3.4:5', memcache_serialization_support='1', memcache_max_connections='4', - section='memcache'): + section='memcache', + item_size_warning_threshold='75'): _srvs = memcache_servers _sers = memcache_serialization_support _maxc = memcache_max_connections _section = section + _warn_threshold = item_size_warning_threshold class SetConfigParser(object): @@ -64,7 +66,7 @@ def get_config_parser(memcache_servers='1.2.3.4:5', 'memcache_servers': memcache_servers, 'memcache_serialization_support': memcache_serialization_support, - 'memcache_max_connections': memcache_max_connections, + 'memcache_max_connections': memcache_max_connections } def read(self, path): @@ -85,6 +87,10 @@ def get_config_parser(memcache_servers='1.2.3.4:5', if _maxc == 'error': raise NoOptionError(option, section) return _maxc + elif option == 'item_size_warning_threshold': + if _warn_threshold == 'error': + raise NoOptionError(option, section) + return _warn_threshold else: raise NoOptionError(option, section) else: @@ -114,12 +120,19 @@ class TestCacheMiddleware(unittest.TestCase): {'memcache_servers': '6.7.8.9:10'}, {'memcache_serialization_support': '0'}, {'memcache_max_connections': '30'}, + {'item_size_warning_threshold': 75}, {'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'} + 'memcache_max_connections': '30'}, + {'memcache_servers': '6.7.8.9:10', + 'item_size_warning_threshold': '75'}, + {'memcache_serialization_support': '0', + 'item_size_warning_threshold': '75'}, + {'item_size_warning_threshold': '75', + 'memcache_max_connections': '30'}, ): with self.assertRaises(Exception) as catcher: memcache.MemcacheMiddleware(FakeApp(), d) @@ -134,7 +147,8 @@ class TestCacheMiddleware(unittest.TestCase): memcache.MemcacheMiddleware( FakeApp(), {'memcache_servers': '1.2.3.4:5', 'memcache_serialization_support': '2', - 'memcache_max_connections': '30'}) + 'memcache_max_connections': '30', + 'item_size_warning_threshold': '80'}) except Exception as err: exc = err self.assertIsNone(exc) @@ -147,6 +161,7 @@ class TestCacheMiddleware(unittest.TestCase): self.assertEqual(app.memcache._allow_unpickle, False) self.assertEqual( app.memcache._client_cache['127.0.0.1:11211'].max_size, 2) + self.assertEqual(app.memcache.item_size_warning_threshold, -1) def test_conf_inline(self): with mock.patch.object(memcache, 'ConfigParser', get_config_parser()): @@ -154,12 +169,14 @@ class TestCacheMiddleware(unittest.TestCase): FakeApp(), {'memcache_servers': '6.7.8.9:10', 'memcache_serialization_support': '0', - 'memcache_max_connections': '5'}) + 'memcache_max_connections': '5', + 'item_size_warning_threshold': '75'}) self.assertEqual(app.memcache_servers, '6.7.8.9:10') self.assertEqual(app.memcache._allow_pickle, True) self.assertEqual(app.memcache._allow_unpickle, True) self.assertEqual( app.memcache._client_cache['6.7.8.9:10'].max_size, 5) + self.assertEqual(app.memcache.item_size_warning_threshold, 75) def test_conf_inline_ratelimiting(self): with mock.patch.object(memcache, 'ConfigParser', get_config_parser()): @@ -235,6 +252,17 @@ class TestCacheMiddleware(unittest.TestCase): self.assertEqual( app.memcache._client_cache['6.7.8.9:10'].max_size, 4) + def test_conf_inline_bad_item_warning_threshold(self): + with mock.patch.object(memcache, 'ConfigParser', get_config_parser()): + with self.assertRaises(ValueError) as err: + memcache.MemcacheMiddleware( + FakeApp(), + {'memcache_servers': '6.7.8.9:10', + 'memcache_serialization_support': '0', + 'item_size_warning_threshold': 'bad42'}) + self.assertIn('invalid literal for int() with base 10:', + str(err.exception)) + def test_conf_from_extra_conf(self): with mock.patch.object(memcache, 'ConfigParser', get_config_parser()): app = memcache.MemcacheMiddleware(FakeApp(), {}) @@ -332,6 +360,7 @@ class TestCacheMiddleware(unittest.TestCase): # tries is limited to server count self.assertEqual(memcache_ring._tries, 1) self.assertEqual(memcache_ring._io_timeout, 2.0) + self.assertEqual(memcache_ring.item_size_warning_threshold, -1) @with_tempdir def test_real_config_with_options(self, tempdir): @@ -351,6 +380,7 @@ class TestCacheMiddleware(unittest.TestCase): tries = 4 io_timeout = 1.0 tls_enabled = true + item_size_warning_threshold = 1000 """ config_path = os.path.join(tempdir, 'test.conf') with open(config_path, 'w') as f: @@ -370,6 +400,7 @@ class TestCacheMiddleware(unittest.TestCase): self.assertIsInstance( list(memcache_ring._client_cache.values())[0]._tls_context, ssl.SSLContext) + self.assertEqual(memcache_ring.item_size_warning_threshold, 1000) @with_tempdir def test_real_memcache_config(self, tempdir): @@ -399,6 +430,7 @@ class TestCacheMiddleware(unittest.TestCase): io_timeout = 1.0 error_suppression_limit = 0 error_suppression_interval = 1.5 + item_size_warning_threshold = 50 """ memcache_config_path = os.path.join(tempdir, 'memcache.conf') with open(memcache_config_path, 'w') as f: @@ -415,6 +447,7 @@ class TestCacheMiddleware(unittest.TestCase): self.assertEqual(memcache_ring._error_limit_count, 0) self.assertEqual(memcache_ring._error_limit_time, 1.5) self.assertEqual(memcache_ring._error_limit_duration, 1.5) + self.assertEqual(memcache_ring.item_size_warning_threshold, 50) if __name__ == '__main__': diff --git a/test/unit/common/test_memcached.py b/test/unit/common/test_memcached.py index 4fafd7913b..06e567d868 100644 --- a/test/unit/common/test_memcached.py +++ b/test/unit/common/test_memcached.py @@ -15,7 +15,6 @@ # limitations under the License. """Tests for swift.common.utils""" - from collections import defaultdict import errno import io @@ -33,7 +32,7 @@ from eventlet import GreenPool, sleep, Queue from eventlet.pools import Pool from swift.common import memcached -from swift.common.utils import md5 +from swift.common.utils import md5, human_readable from mock import patch, MagicMock from test.debug_logger import debug_logger @@ -947,6 +946,56 @@ class TestMemcached(unittest.TestCase): self.assertEqual(1, mock_sock.close.call_count) + def test_item_size_warning_threshold(self): + mock = MockMemcached() + mocked_pool = MockedMemcachePool([(mock, mock)] * 2) + + def do_test(d, threshold, should_warn, error=False): + self.logger.clear() + try: + memcache_client = memcached.MemcacheRing( + ['1.2.3.4:11211'], item_size_warning_threshold=threshold, + logger=self.logger) + memcache_client._client_cache['1.2.3.4:11211'] = mocked_pool + memcache_client.set('some_key', d, serialize=False) + warning_lines = self.logger.get_lines_for_level('warning') + if should_warn: + self.assertIn( + 'Item size larger than warning threshold: ' + '%d (%s) >= %d (%s)' % ( + len(d), human_readable(len(d)), threshold, + human_readable(threshold)), + warning_lines[0]) + else: + self.assertFalse(warning_lines) + except ValueError as err: + if not err: + self.fail(err) + else: + self.assertIn( + 'Config option must be a number, greater than 0, ' + 'less than 100, not "%s".' % threshold, + str(err)) + + data = '1' * 100 + # let's start with something easy, say warning at 80 + for data_size, warn in ((79, False), (80, True), (81, True), + (99, True), (100, True)): + do_test(data[:data_size], 80, warn) + + # if we set the threshold to -1 will turn off the warning + for data_size, warn in ((79, False), (80, False), (81, False), + (99, False), (100, False)): + do_test(data[:data_size], -1, warn) + + # Changing to 0 should warn on everything + for data_size, warn in ((0, True), (1, True), (50, True), + (99, True), (100, True)): + do_test(data[:data_size], 0, warn) + + # Let's do a big number + do_test('1' * 2048576, 1000000, True) + if __name__ == '__main__': unittest.main()