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

Commit

Permalink
Create a new more specific error for the cases where a slot is not co…
Browse files Browse the repository at this point in the history
…vered by the cluster. If this happens it will attempt to rebuild the cluster until TTL is expired and then raise that exception back to the user. Fixes #350
  • Loading branch information
Grokzen committed May 22, 2020
1 parent bf01104 commit 76aa25f
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 3 deletions.
1 change: 1 addition & 0 deletions docs/release-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Release Notes
* Nodemanager initiailize should now handle usernames properly (#365)
* PubSub tests has been all been disabled
* New feature, host_port_remap. Send in a remapping configuration to RedisCluster instance where the nodes configuration recieved from the redis cluster can be altered to allow for connection in certain circumstances. See new section in clients.rst in docs/ for usage example.
* When a slot is not covered by the cluster, it will not raise SlotNotCoveredError instead of the old generic RedisClusterException. The client will not attempt to rebuild the cluster layout a few times before giving up and raising that exception to the user. (#350)


2.0.0 (Aug 12, 2019)
Expand Down
5 changes: 5 additions & 0 deletions docs/upgrading.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ This document describes what must be done when upgrading between different versi

Python3 version must now be one of 3.5, 3.6, 3.7, 3.8

The following exception example has now a new more specific exception class that will be attempted to be caught and the client to resolve the cluster layout. If enough attempts has been made then SlotNotCoveredError will be raised with the same message as before. If you have catch for RedisClusterException you either remove it and let the client try to resolve the cluster layout itself, or start to catch SlotNotCoveredError. This error usually happens during failover if you run skip_full_coverage_check=True when running on AWS ElasticCache for example.

## Example exception
rediscluster.exceptions.RedisClusterException: Slot "6986" not covered by the cluster. "skip_full_coverage_check=True"


1.3.x --> 2.0.0
---------------
Expand Down
11 changes: 11 additions & 0 deletions rediscluster/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
ClusterError,
MovedError,
RedisClusterException,
SlotNotCoveredError,
TryAgainError,
)
from .pubsub import ClusterPubSub
Expand Down Expand Up @@ -595,6 +596,16 @@ def _execute_command(self, *args, **kwargs):

connection.send_command(*args)
return self.parse_response(connection, command, **kwargs)
except SlotNotCoveredError as e:
# In some cases during failover to a replica is happening
# a slot sometimes is not covered by the cluster layout and
# we need to attempt to refresh the cluster layout and try again
self.refresh_table_asap = True
time.sleep(0.05)

# This is the last attempt before we run out of TTL, raise the exception
if ttl == 1:
raise e
except (RedisClusterException, BusyLoadingError):
raise
except ConnectionError:
Expand Down
4 changes: 2 additions & 2 deletions rediscluster/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from .exceptions import (
RedisClusterException, AskError, MovedError,
TryAgainError, ClusterDownError, ClusterCrossSlotError,
MasterDownError,
MasterDownError, SlotNotCoveredError,
)

# 3rd party imports
Expand Down Expand Up @@ -329,7 +329,7 @@ def get_master_node_by_slot(self, slot):
try:
return self.nodes.slots[slot][0]
except KeyError as ke:
raise RedisClusterException('Slot "{slot}" not covered by the cluster. "skip_full_coverage_check={skip_full_coverage_check}"'.format(
raise SlotNotCoveredError('Slot "{slot}" not covered by the cluster. "skip_full_coverage_check={skip_full_coverage_check}"'.format(
slot=slot, skip_full_coverage_check=self.nodes._skip_full_coverage_check,
))

Expand Down
11 changes: 11 additions & 0 deletions rediscluster/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,14 @@ class MasterDownError(ClusterDownError):
"""
"""
pass


class SlotNotCoveredError(RedisClusterException):
"""
This error only happens in the case where the connection pool will try to
fetch what node that is covered by a given slot.
If this error is raised the client should drop the current node layout and
attempt to reconnect and refresh the node layout again
"""
pass
7 changes: 6 additions & 1 deletion tests/test_cluster_connection_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from rediscluster.connection import (
ClusterConnectionPool, ClusterBlockingConnectionPool, ClusterReadOnlyConnectionPool,
ClusterConnection, UnixDomainSocketConnection)
from rediscluster.exceptions import RedisClusterException
from rediscluster.exceptions import RedisClusterException, SlotNotCoveredError
from .conftest import (skip_if_server_version_lt, skip_for_no_cluster_impl)

# 3rd party imports
Expand Down Expand Up @@ -205,6 +205,11 @@ def test_master_node_by_slot(self):
node = pool.get_master_node_by_slot(12182)
node['port'] = 7002

pool = self.get_pool(connection_kwargs={})
pool.nodes.slots = {}
with pytest.raises(SlotNotCoveredError):
pool.get_master_node_by_slot(12182)

def test_from_url_connection_classes(self):
from rediscluster.client import RedisCluster
from rediscluster.connection import ClusterConnectionPool, ClusterConnection, SSLClusterConnection
Expand Down

0 comments on commit 76aa25f

Please sign in to comment.