Skip to content

FIX #651 try next Sentinel node on TimeoutError #684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions redis/sentinel.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

from redis.client import StrictRedis
from redis.connection import ConnectionPool, Connection
from redis.exceptions import ConnectionError, ResponseError, ReadOnlyError
from redis.exceptions import (ConnectionError, ResponseError, ReadOnlyError,
TimeoutError)
from redis._compat import iteritems, nativestr, xrange


Expand Down Expand Up @@ -211,7 +212,7 @@ def discover_master(self, service_name):
for sentinel_no, sentinel in enumerate(self.sentinels):
try:
masters = sentinel.sentinel_masters()
except ConnectionError:
except (ConnectionError, TimeoutError):
continue
state = masters.get(service_name)
if state and self.check_master_state(state, service_name):
Expand All @@ -235,7 +236,7 @@ def discover_slaves(self, service_name):
for sentinel in self.sentinels:
try:
slaves = sentinel.sentinel_slaves(service_name)
except (ConnectionError, ResponseError):
except (ConnectionError, ResponseError, TimeoutError):
continue
slaves = self.filter_slaves(slaves)
if slaves:
Expand Down
22 changes: 22 additions & 0 deletions tests/test_sentinel.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ def __init__(self, cluster, id):

def sentinel_masters(self):
self.cluster.connection_error_if_down(self)
self.cluster.timeout_if_down(self)
return {self.cluster.service_name: self.cluster.master}

def sentinel_slaves(self, master_name):
self.cluster.connection_error_if_down(self)
self.cluster.timeout_if_down(self)
if master_name != self.cluster.service_name:
return []
return self.cluster.slaves
Expand All @@ -38,11 +40,16 @@ def __init__(self, service_name='mymaster', ip='127.0.0.1', port=6379):
self.service_name = service_name
self.slaves = []
self.nodes_down = set()
self.nodes_timeout = set()

def connection_error_if_down(self, node):
if node.id in self.nodes_down:
raise exceptions.ConnectionError

def timeout_if_down(self, node):
if node.id in self.nodes_timeout:
raise exceptions.TimeoutError

def client(self, host, port, **kwargs):
return SentinelTestClient(self, (host, port))

Expand Down Expand Up @@ -82,6 +89,15 @@ def test_discover_master_sentinel_down(cluster, sentinel):
assert sentinel.sentinels[0].id == ('bar', 26379)


def test_discover_master_sentinel_timeout(cluster, sentinel):
# Put first sentinel 'foo' down
cluster.nodes_timeout.add(('foo', 26379))
address = sentinel.discover_master('mymaster')
assert address == ('127.0.0.1', 6379)
# 'bar' is now first sentinel
assert sentinel.sentinels[0].id == ('bar', 26379)


def test_master_min_other_sentinels(cluster):
sentinel = Sentinel([('foo', 26379)], min_other_sentinels=1)
# min_other_sentinels
Expand Down Expand Up @@ -130,6 +146,12 @@ def test_discover_slaves(cluster, sentinel):
cluster.nodes_down.add(('foo', 26379))
assert sentinel.discover_slaves('mymaster') == [
('slave0', 1234), ('slave1', 1234)]
cluster.nodes_down.clear()

# node0 -> TIMEOUT
cluster.nodes_timeout.add(('foo', 26379))
assert sentinel.discover_slaves('mymaster') == [
('slave0', 1234), ('slave1', 1234)]


def test_master_for(cluster, sentinel):
Expand Down