From 08dfd40e1950945b10a81c9a8fa22e5cc43cce83 Mon Sep 17 00:00:00 2001 From: Hector Castro Date: Sat, 21 Jan 2017 13:53:24 -0500 Subject: [PATCH 1/4] Add support for when cluster endpoint is not available Modify `get_cluster_info` to include a timeout (defaults to 3 seconds) and gracefully degrade when the ElastiCache cluster configuration endpoint isn't available by returning the provided `host` and `port` in the `nodes` list. I also added an additional test to the protocol test suite to cover the additional functionality. --- django_elasticache/cluster_utils.py | 16 +++++++++++++--- tests/test_protocol.py | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/django_elasticache/cluster_utils.py b/django_elasticache/cluster_utils.py index 7d4a11d..17d2454 100644 --- a/django_elasticache/cluster_utils.py +++ b/django_elasticache/cluster_utils.py @@ -17,7 +17,7 @@ def __init__(self, cmd, response): 'Unexpected response {} for command {}'.format(response, cmd)) -def get_cluster_info(host, port): +def get_cluster_info(host, port, timeout=3): """ return dict with info about nodes in cluster and current version { @@ -30,7 +30,7 @@ def get_cluster_info(host, port): """ client = Telnet(host, int(port)) client.write(b'version\n') - res = client.read_until(b'\r\n').strip() + res = client.read_until(b'\r\n', timeout).strip() version_list = res.split(b' ') if len(version_list) not in [2, 3] or version_list[0] != b'VERSION': raise WrongProtocolData('version', res) @@ -40,8 +40,18 @@ def get_cluster_info(host, port): else: cmd = b'get AmazonElastiCache:cluster\n' client.write(cmd) - res = client.read_until(b'\n\r\nEND\r\n') + res = client.read_until(b'\n\r\nEND\r\n', timeout) client.close() + + if res == 'ERROR\r\n': + return { + 'version': version, + 'nodes': [ + '{}:{}'.format(smart_text(host), + smart_text(port)) + ] + } + ls = list(filter(None, re.compile(br'\r?\n').split(res))) if len(ls) != 4: raise WrongProtocolData(cmd, res) diff --git a/tests/test_protocol.py b/tests/test_protocol.py index bf8d500..69f3bb6 100644 --- a/tests/test_protocol.py +++ b/tests/test_protocol.py @@ -23,6 +23,11 @@ b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n', ] +TEST_PROTOCOL_4 = [ + b'VERSION 1.4.34', + b'ERROR\r\n', +] + @patch('django_elasticache.cluster_utils.Telnet') def test_happy_path(Telnet): @@ -75,3 +80,16 @@ def test_ubuntu_protocol(Telnet): call(b'version\n'), call(b'config get cluster\n'), ]) + + +@patch('django_elasticache.cluster_utils.Telnet') +def test_no_configuration_protocol_support(Telnet): + client = Telnet.return_value + client.read_until.side_effect = TEST_PROTOCOL_4 + info = get_cluster_info('test', 0) + client.write.assert_has_calls([ + call(b'version\n'), + call(b'config get cluster\n'), + ]) + eq_(info['version'], '1.4.34') + eq_(info['nodes'], ['test:0']) From 49882896c99b4dd2401b8581d896798c755a0992 Mon Sep 17 00:00:00 2001 From: Hector Castro Date: Sun, 29 Jan 2017 22:01:49 -0500 Subject: [PATCH 2/4] Use Telnet.expect() to test for success and failure Make use of Telnet.expect() to match against the success (END) and failure case (ERROR) in the same call. Update protocol tests to handle patching expect() with output that was previously returned by read_until(). --- django_elasticache/cluster_utils.py | 7 +++-- tests/test_protocol.py | 43 ++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/django_elasticache/cluster_utils.py b/django_elasticache/cluster_utils.py index 17d2454..cfab499 100644 --- a/django_elasticache/cluster_utils.py +++ b/django_elasticache/cluster_utils.py @@ -40,10 +40,13 @@ def get_cluster_info(host, port, timeout=3): else: cmd = b'get AmazonElastiCache:cluster\n' client.write(cmd) - res = client.read_until(b'\n\r\nEND\r\n', timeout) + regex_index, match_object, res = client.expect([ + re.compile(b'\n\r\nEND\r\n'), + re.compile(b'ERROR\r\n') + ], timeout) client.close() - if res == 'ERROR\r\n': + if res == b'ERROR\r\n': return { 'version': version, 'nodes': [ diff --git a/tests/test_protocol.py b/tests/test_protocol.py index 69f3bb6..6aca4eb 100644 --- a/tests/test_protocol.py +++ b/tests/test_protocol.py @@ -8,31 +8,44 @@ from unittest.mock import patch, call, MagicMock -TEST_PROTOCOL_1 = [ +TEST_PROTOCOL_1_READ_UNTIL = [ b'VERSION 1.4.14', - b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n', ] -TEST_PROTOCOL_2 = [ +TEST_PROTOCOL_1_EXPECT = [ + (0, None, b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n'), # NOQA +] + +TEST_PROTOCOL_2_READ_UNTIL = [ b'VERSION 1.4.13', - b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n', ] -TEST_PROTOCOL_3 = [ +TEST_PROTOCOL_2_EXPECT = [ + (0, None, b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n'), # NOQA +] + +TEST_PROTOCOL_3_READ_UNTIL = [ b'VERSION 1.4.14 (Ubuntu)', - b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n', ] -TEST_PROTOCOL_4 = [ +TEST_PROTOCOL_3_EXPECT = [ + (0, None, b'CONFIG cluster 0 138\r\n1\nhost|ip|port host||port\n\r\nEND\r\n'), # NOQA +] + +TEST_PROTOCOL_4_READ_UNTIL = [ b'VERSION 1.4.34', - b'ERROR\r\n', +] + +TEST_PROTOCOL_4_EXPECT = [ + (0, None, b'ERROR\r\n'), ] @patch('django_elasticache.cluster_utils.Telnet') def test_happy_path(Telnet): client = Telnet.return_value - client.read_until.side_effect = TEST_PROTOCOL_1 + client.read_until.side_effect = TEST_PROTOCOL_1_READ_UNTIL + client.expect.side_effect = TEST_PROTOCOL_1_EXPECT info = get_cluster_info('', 0) eq_(info['version'], 1) eq_(info['nodes'], ['ip:port', 'host:port']) @@ -47,7 +60,8 @@ def test_bad_protocol(): @patch('django_elasticache.cluster_utils.Telnet') def test_last_versions(Telnet): client = Telnet.return_value - client.read_until.side_effect = TEST_PROTOCOL_1 + client.read_until.side_effect = TEST_PROTOCOL_1_READ_UNTIL + client.expect.side_effect = TEST_PROTOCOL_1_EXPECT get_cluster_info('', 0) client.write.assert_has_calls([ call(b'version\n'), @@ -58,7 +72,8 @@ def test_last_versions(Telnet): @patch('django_elasticache.cluster_utils.Telnet') def test_prev_versions(Telnet): client = Telnet.return_value - client.read_until.side_effect = TEST_PROTOCOL_2 + client.read_until.side_effect = TEST_PROTOCOL_2_READ_UNTIL + client.expect.side_effect = TEST_PROTOCOL_2_EXPECT get_cluster_info('', 0) client.write.assert_has_calls([ call(b'version\n'), @@ -69,7 +84,8 @@ def test_prev_versions(Telnet): @patch('django_elasticache.cluster_utils.Telnet') def test_ubuntu_protocol(Telnet): client = Telnet.return_value - client.read_until.side_effect = TEST_PROTOCOL_3 + client.read_until.side_effect = TEST_PROTOCOL_3_READ_UNTIL + client.expect.side_effect = TEST_PROTOCOL_3_EXPECT try: get_cluster_info('', 0) @@ -85,7 +101,8 @@ def test_ubuntu_protocol(Telnet): @patch('django_elasticache.cluster_utils.Telnet') def test_no_configuration_protocol_support(Telnet): client = Telnet.return_value - client.read_until.side_effect = TEST_PROTOCOL_4 + client.read_until.side_effect = TEST_PROTOCOL_4_READ_UNTIL + client.expect.side_effect = TEST_PROTOCOL_4_EXPECT info = get_cluster_info('test', 0) client.write.assert_has_calls([ call(b'version\n'), From 4cd11ac595fd2c374fd92963b974102a521960ac Mon Sep 17 00:00:00 2001 From: Hector Castro Date: Sat, 4 Feb 2017 10:33:31 -0500 Subject: [PATCH 3/4] Add support for IGNORE_CLUSTER_ERRORS option Conditionally ignore failures to `config get cluster` calls against the configured LOCATION endpoint with the introduction of a `IGNORE_CLUSTER_ERRORS` option (that defaults to `False`). --- README.rst | 9 ++++++++- django_elasticache/cluster_utils.py | 4 ++-- django_elasticache/memcached.py | 9 +++++++-- tests/test_backend.py | 5 +++-- tests/test_protocol.py | 17 +++++++++++++++-- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/README.rst b/README.rst index f6b6386..644a037 100644 --- a/README.rst +++ b/README.rst @@ -36,10 +36,13 @@ Your cache backend should look something like this:: 'default': { 'BACKEND': 'django_elasticache.memcached.ElastiCache', 'LOCATION': 'cache-c.draaaf.cfg.use1.cache.amazonaws.com:11211', + 'OPTIONS' { + 'IGNORE_CLUSTER_ERRORS': [True,False], + }, } } -By the first call to cache it connects to cluster (using LOCATION param), +By the first call to cache it connects to cluster (using ``LOCATION`` param), gets list of all nodes and setup pylibmc client using full list of nodes. As result your cache will work with all nodes in cluster and automatically detect new nodes in cluster. List of nodes are stored in class-level @@ -48,6 +51,10 @@ But if you're using gunicorn or mod_wsgi you usually have max_request settings w restart process after some count of processed requests, so auto discovery will work fine. +The ``IGNORE_CLUSTER_ERRORS`` option is useful when ``LOCATION`` doesn't have support +for ``config get cluster``. When set to ``True``, and ``config get cluster`` fails, +it returns a list of a single node with the same endpoint supplied to ``LOCATION``. + Django-elasticache changes default pylibmc params to increase performance. Another solutions diff --git a/django_elasticache/cluster_utils.py b/django_elasticache/cluster_utils.py index cfab499..a046990 100644 --- a/django_elasticache/cluster_utils.py +++ b/django_elasticache/cluster_utils.py @@ -17,7 +17,7 @@ def __init__(self, cmd, response): 'Unexpected response {} for command {}'.format(response, cmd)) -def get_cluster_info(host, port, timeout=3): +def get_cluster_info(host, port, ignore_cluster_errors=False, timeout=3): """ return dict with info about nodes in cluster and current version { @@ -46,7 +46,7 @@ def get_cluster_info(host, port, timeout=3): ], timeout) client.close() - if res == b'ERROR\r\n': + if res == b'ERROR\r\n' and ignore_cluster_errors: return { 'version': version, 'nodes': [ diff --git a/django_elasticache/memcached.py b/django_elasticache/memcached.py index 0f8a8be..e755937 100644 --- a/django_elasticache/memcached.py +++ b/django_elasticache/memcached.py @@ -38,6 +38,9 @@ def __init__(self, server, params): raise InvalidCacheBackendError( 'Server configuration should be in format IP:port') + self._ignore_cluster_errors = self._options.get( + 'IGNORE_CLUSTER_ERRORS', False) + def update_params(self, params): """ update connection params to maximize performance @@ -51,7 +54,8 @@ def update_params(self, params): # set special 'behaviors' pylibmc attributes params['OPTIONS'] = { 'tcp_nodelay': True, - 'ketama': True + 'ketama': True, + 'IGNORE_CLUSTER_ERRORS': False, } def clear_cluster_nodes_cache(self): @@ -67,7 +71,8 @@ def get_cluster_nodes(self): server, port = self._servers[0].split(':') try: self._cluster_nodes_cache = ( - get_cluster_info(server, port)['nodes']) + get_cluster_info(server, port, + self._ignore_cluster_errors)['nodes']) except (socket.gaierror, socket.timeout) as err: raise Exception('Cannot connect to cluster {} ({})'.format( self._servers[0], err diff --git a/tests/test_backend.py b/tests/test_backend.py index 3006652..774ecdb 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -20,6 +20,7 @@ def test_patch_params(): eq_(params['BINARY'], True) eq_(params['OPTIONS']['tcp_nodelay'], True) eq_(params['OPTIONS']['ketama'], True) + eq_(params['OPTIONS']['IGNORE_CLUSTER_ERRORS'], False) @raises(Exception) @@ -47,7 +48,7 @@ def test_split_servers(get_cluster_info): } backend._lib.Client = Mock() assert backend._cache - get_cluster_info.assert_called_once_with('h', '0') + get_cluster_info.assert_called_once_with('h', '0', False) backend._lib.Client.assert_called_once_with(servers) @@ -70,7 +71,7 @@ def test_node_info_cache(get_cluster_info): eq_(backend._cache.get.call_count, 2) eq_(backend._cache.set.call_count, 2) - get_cluster_info.assert_called_once_with('h', '0') + get_cluster_info.assert_called_once_with('h', '0', False) @patch('django.conf.settings', global_settings) diff --git a/tests/test_protocol.py b/tests/test_protocol.py index 6aca4eb..7232a1a 100644 --- a/tests/test_protocol.py +++ b/tests/test_protocol.py @@ -99,14 +99,27 @@ def test_ubuntu_protocol(Telnet): @patch('django_elasticache.cluster_utils.Telnet') -def test_no_configuration_protocol_support(Telnet): +def test_no_configuration_protocol_support_with_errors_ignored(Telnet): client = Telnet.return_value client.read_until.side_effect = TEST_PROTOCOL_4_READ_UNTIL client.expect.side_effect = TEST_PROTOCOL_4_EXPECT - info = get_cluster_info('test', 0) + info = get_cluster_info('test', 0, ignore_cluster_errors=True) client.write.assert_has_calls([ call(b'version\n'), call(b'config get cluster\n'), ]) eq_(info['version'], '1.4.34') eq_(info['nodes'], ['test:0']) + + +@raises(WrongProtocolData) +@patch('django_elasticache.cluster_utils.Telnet') +def test_no_configuration_protocol_support_with_errors(Telnet): + client = Telnet.return_value + client.read_until.side_effect = TEST_PROTOCOL_4_READ_UNTIL + client.expect.side_effect = TEST_PROTOCOL_4_EXPECT + get_cluster_info('test', 0, ignore_cluster_errors=False) + client.write.assert_has_calls([ + call(b'version\n'), + call(b'config get cluster\n'), + ]) From 384dd31e2021013b3e0100cda8711329fe0cfa7f Mon Sep 17 00:00:00 2001 From: Hector Castro Date: Sat, 4 Feb 2017 14:25:15 -0500 Subject: [PATCH 4/4] Remove timeout; keep IGNORE_CLUSTER_ERRORS within this backend --- django_elasticache/cluster_utils.py | 6 +++--- django_elasticache/memcached.py | 3 +-- tests/test_backend.py | 1 - 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/django_elasticache/cluster_utils.py b/django_elasticache/cluster_utils.py index a046990..740e4cf 100644 --- a/django_elasticache/cluster_utils.py +++ b/django_elasticache/cluster_utils.py @@ -17,7 +17,7 @@ def __init__(self, cmd, response): 'Unexpected response {} for command {}'.format(response, cmd)) -def get_cluster_info(host, port, ignore_cluster_errors=False, timeout=3): +def get_cluster_info(host, port, ignore_cluster_errors=False): """ return dict with info about nodes in cluster and current version { @@ -30,7 +30,7 @@ def get_cluster_info(host, port, ignore_cluster_errors=False, timeout=3): """ client = Telnet(host, int(port)) client.write(b'version\n') - res = client.read_until(b'\r\n', timeout).strip() + res = client.read_until(b'\r\n').strip() version_list = res.split(b' ') if len(version_list) not in [2, 3] or version_list[0] != b'VERSION': raise WrongProtocolData('version', res) @@ -43,7 +43,7 @@ def get_cluster_info(host, port, ignore_cluster_errors=False, timeout=3): regex_index, match_object, res = client.expect([ re.compile(b'\n\r\nEND\r\n'), re.compile(b'ERROR\r\n') - ], timeout) + ]) client.close() if res == b'ERROR\r\n' and ignore_cluster_errors: diff --git a/django_elasticache/memcached.py b/django_elasticache/memcached.py index e755937..f7c19fc 100644 --- a/django_elasticache/memcached.py +++ b/django_elasticache/memcached.py @@ -54,8 +54,7 @@ def update_params(self, params): # set special 'behaviors' pylibmc attributes params['OPTIONS'] = { 'tcp_nodelay': True, - 'ketama': True, - 'IGNORE_CLUSTER_ERRORS': False, + 'ketama': True } def clear_cluster_nodes_cache(self): diff --git a/tests/test_backend.py b/tests/test_backend.py index 774ecdb..9b6f425 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -20,7 +20,6 @@ def test_patch_params(): eq_(params['BINARY'], True) eq_(params['OPTIONS']['tcp_nodelay'], True) eq_(params['OPTIONS']['ketama'], True) - eq_(params['OPTIONS']['IGNORE_CLUSTER_ERRORS'], False) @raises(Exception)