Skip to content
This repository was archived by the owner on Jan 9, 2024. It is now read-only.

Commit 7c87352

Browse files
committed
Initialise the node manager correctly
populate_startup_nodes is meant to put all the known nodes in the startup nodes list, but when it was called, the nodes list it was using (self.nodes) is not populated yet. So populate startup nodes after initialising self.nodes, otherwise when we choose a random connection (which is chosen from startup nodes), the options are too limited. Often a user will specify a single startup node at the beginning.
1 parent 6b4dbf3 commit 7c87352

File tree

3 files changed

+15
-4
lines changed

3 files changed

+15
-4
lines changed

rediscluster/nodemanager.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ def initialize(self):
306306
# Set the tmp variables to the real variables
307307
self.slots = tmp_slots
308308
self.nodes = nodes_cache
309+
self.populate_startup_nodes()
309310
self.reinitialize_counter = 0
310311

311312
log.debug("NodeManager initialize done : Nodes")
@@ -413,7 +414,8 @@ def set_node(self, host, port, server_type=None):
413414

414415
def populate_startup_nodes(self):
415416
"""
416-
Do something with all startup nodes and filters out any duplicates
417+
Use nodes to populate startup_nodes, so that we have more chances
418+
if a subset of the cluster fails.
417419
"""
418420
for item in self.startup_nodes:
419421
self.set_node_name(item)

tests/test_cluster_node_manager.py

+12-2
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ def test_empty_startup_nodes():
169169

170170
def test_wrong_startup_nodes_type():
171171
"""
172-
If something other then a list type itteratable is provided it should fail
172+
If something other then a list type iterable is provided it should fail
173173
"""
174174
with pytest.raises(RedisClusterException):
175175
NodeManager({})
@@ -263,9 +263,19 @@ def test_all_nodes():
263263
assert node in nodes
264264

265265

266+
def test_startup_nodes_are_populated():
267+
"""
268+
Set a list of nodes and it should be possible to iterate over all
269+
"""
270+
n = NodeManager(startup_nodes=[{"host": "127.0.0.1", "port": 7000}])
271+
n.initialize()
272+
273+
assert sorted([node['port'] for node in n.startup_nodes]) == [7000, 7000, 7001, 7002, 7003, 7004, 7005]
274+
275+
266276
def test_all_nodes_masters():
267277
"""
268-
Set a list of nodes with random masters/slaves config and it shold be possible
278+
Set a list of nodes with random masters/slaves config and it should be possible
269279
to iterate over all of them.
270280
"""
271281
n = NodeManager(

tests/test_multiprocessing_cluster.py

-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ def target(pool):
117117

118118
# Check that connection is still alive after fork process has exited
119119
# and disconnected the connections in its pool
120-
conn = pool.get_random_connection()
121120
with exit_callback(pool.release, conn):
122121
assert conn.send_command('ping') is None
123122
assert conn.read_response() == b'PONG'

0 commit comments

Comments
 (0)