From 5a0579bcf685cf71bbaa8772d03a01be68655690 Mon Sep 17 00:00:00 2001 From: Grokzen Date: Sat, 10 Oct 2015 19:16:10 +0200 Subject: [PATCH] Fix broken tests by use the same reinitialize steps logic as in normal client --- rediscluster/client.py | 3 ++- rediscluster/pipeline.py | 10 +++++++++- tests/conftest.py | 2 +- tests/test_cluster_obj.py | 15 +++++++-------- tests/test_pipeline.py | 4 ++-- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/rediscluster/client.py b/rediscluster/client.py index d04c65fe..b3ed6739 100644 --- a/rediscluster/client.py +++ b/rediscluster/client.py @@ -156,7 +156,8 @@ def pipeline(self, transaction=None, shard_hint=None, use_threads=None): nodes_callbacks=self.nodes_callbacks, result_callbacks=self.result_callbacks, response_callbacks=self.response_callbacks, - use_threads=self.pipeline_use_threads if use_threads is None else use_threads + use_threads=self.pipeline_use_threads if use_threads is None else use_threads, + reinitialize_steps=self.reinitialize_steps, ) def transaction(self, func, *watches, **kwargs): diff --git a/rediscluster/pipeline.py b/rediscluster/pipeline.py index 4644f03f..bc14a8f6 100644 --- a/rediscluster/pipeline.py +++ b/rediscluster/pipeline.py @@ -26,7 +26,7 @@ class StrictClusterPipeline(StrictRedisCluster): def __init__(self, connection_pool, nodes_callbacks=None, result_callbacks=None, response_callbacks=None, startup_nodes=None, refresh_table_asap=False, - use_threads=True): + use_threads=True, reinitialize_steps=None): self.connection_pool = connection_pool self.startup_nodes = startup_nodes if startup_nodes else [] self.refresh_table_asap = refresh_table_asap @@ -37,6 +37,9 @@ def __init__(self, connection_pool, nodes_callbacks=None, result_callbacks=None, self.response_callbacks = response_callbacks self.use_threads = use_threads + self.reinitialize_counter = 0 + self.reinitialize_steps = reinitialize_steps or 25 + def __repr__(self): return "{}".format(type(self).__name__) @@ -211,6 +214,11 @@ def send_cluster_commands(self, stack, raise_on_error=True, allow_redirections=T raise v if isinstance(v, MovedError): + # Do not perform full cluster refresh on every MOVED error + self.reinitialize_counter += 1 + if self.reinitialize_counter % self.reinitialize_steps == 0: + self.refresh_table_asap = True + node = self.connection_pool.nodes.set_node(v.host, v.port, server_type='master') self.connection_pool.nodes.slots[v.slot_id][0] = node attempt.append(i) diff --git a/tests/conftest.py b/tests/conftest.py index 50dbd9fc..4f7ce05e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -124,7 +124,7 @@ def sr(request, *args, **kwargs): """ Returns a instance of StrictRedisCluster """ - return _init_client(request, cls=StrictRedisCluster, **kwargs) + return _init_client(request, reinitialize_steps=1, cls=StrictRedisCluster, **kwargs) @pytest.fixture() diff --git a/tests/test_cluster_obj.py b/tests/test_cluster_obj.py index fe9c108d..eb7d7611 100644 --- a/tests/test_cluster_obj.py +++ b/tests/test_cluster_obj.py @@ -3,6 +3,7 @@ # python std lib from __future__ import with_statement import re +import time # rediscluster imports from rediscluster import StrictRedisCluster @@ -320,10 +321,10 @@ def ok_response(connection, command_name, **options): def assert_moved_redirection_on_slave(sr, connection_pool_cls, cluster_obj): - + """ + """ # we assume this key is set on 127.0.0.1:7000(7003) sr.set('foo16706', 'foo') - import time time.sleep(1) with patch.object(connection_pool_cls, 'get_node_by_slot') as return_slave_mock: @@ -335,10 +336,8 @@ def assert_moved_redirection_on_slave(sr, connection_pool_cls, cluster_obj): } master_value = {'host': '127.0.0.1', 'name': '127.0.0.1:7000', 'port': 7000, 'server_type': 'master'} - with patch.object( - ClusterConnectionPool, - 'get_master_node_by_slot', - return_value=master_value) as return_master_mock: + with patch.object(ClusterConnectionPool, 'get_master_node_by_slot') as return_master_mock: + return_master_mock.return_value = master_value assert cluster_obj.get('foo16706') == b('foo') assert return_master_mock.call_count == 1 @@ -351,7 +350,7 @@ def test_moved_redirection_on_slave_with_default_client(sr): assert_moved_redirection_on_slave( sr, ClusterConnectionPool, - StrictRedisCluster(host="127.0.0.1", port=7000) + StrictRedisCluster(host="127.0.0.1", port=7000, reinitialize_steps=1) ) @@ -362,7 +361,7 @@ def test_moved_redirection_on_slave_with_readonly_mode_client(sr): assert_moved_redirection_on_slave( sr, ClusterReadOnlyConnectionPool, - StrictRedisCluster(host="127.0.0.1", port=7000, readonly_mode=True) + StrictRedisCluster(host="127.0.0.1", port=7000, readonly_mode=True, reinitialize_steps=1) ) diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 64ac5912..5f90c34f 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -517,7 +517,7 @@ def test_moved_redirection_on_slave_with_default(self): """ self.assert_moved_redirection_on_slave( ClusterConnectionPool, - StrictRedisCluster(host="127.0.0.1", port=7000) + StrictRedisCluster(host="127.0.0.1", port=7000, reinitialize_steps=1) ) def test_moved_redirection_on_slave_with_readonly_mode_client(self, sr): @@ -526,7 +526,7 @@ def test_moved_redirection_on_slave_with_readonly_mode_client(self, sr): """ self.assert_moved_redirection_on_slave( ClusterReadOnlyConnectionPool, - StrictRedisCluster(host="127.0.0.1", port=7000, readonly_mode=True) + StrictRedisCluster(host="127.0.0.1", port=7000, readonly_mode=True, reinitialize_steps=1) ) def test_access_correct_slave_with_readonly_mode_client(self, sr):