-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Support client side caching with RedisCluster #3102
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
Changes from all commits
3842e27
3e36bdb
f0fea7d
23ad064
f0707b4
5f91aee
2bf4c14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -227,6 +227,10 @@ def __init__( | |
_cache = None | ||
self.client_cache = client_cache if client_cache is not None else _cache | ||
if self.client_cache is not None: | ||
if self.protocol not in [3, "3"]: | ||
raise RedisError( | ||
"client caching is only supported with protocol version 3 or higher" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chayim As I remember we agreed to force RESP3 instead of raising an error? Error should be thrown after it |
||
) | ||
self.cache_blacklist = cache_blacklist | ||
self.cache_whitelist = cache_whitelist | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -167,6 +167,13 @@ def parse_cluster_myshardid(resp, **options): | |
"ssl_password", | ||
"unix_socket_path", | ||
"username", | ||
"cache_enable", | ||
"client_cache", | ||
"cache_max_size", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO I think we have enough duplication with all these. Part of another issue to clean up / tech debt? |
||
"cache_ttl", | ||
"cache_eviction_policy", | ||
"cache_blacklist", | ||
"cache_whitelist", | ||
) | ||
KWARGS_DISABLED_KEYS = ("host", "port") | ||
|
||
|
@@ -1060,7 +1067,6 @@ def execute_command(self, *args, **kwargs): | |
list<ClusterNode> | ||
dict<Any, ClusterNode> | ||
""" | ||
kwargs.pop("keys", None) # the keys are used only for client side caching | ||
target_nodes_specified = False | ||
is_default_node = False | ||
target_nodes = None | ||
|
@@ -1119,6 +1125,7 @@ def _execute_command(self, target_node, *args, **kwargs): | |
""" | ||
Send a command to a node in the cluster | ||
""" | ||
keys = kwargs.pop("keys", None) | ||
command = args[0] | ||
redis_node = None | ||
connection = None | ||
|
@@ -1147,14 +1154,18 @@ def _execute_command(self, target_node, *args, **kwargs): | |
connection.send_command("ASKING") | ||
redis_node.parse_response(connection, "ASKING", **kwargs) | ||
asking = False | ||
|
||
connection.send_command(*args) | ||
response = redis_node.parse_response(connection, command, **kwargs) | ||
if command in self.cluster_response_callbacks: | ||
response = self.cluster_response_callbacks[command]( | ||
response, **kwargs | ||
) | ||
return response | ||
response_from_cache = connection._get_from_local_cache(args) | ||
if response_from_cache is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
return response_from_cache | ||
else: | ||
connection.send_command(*args) | ||
response = redis_node.parse_response(connection, command, **kwargs) | ||
if command in self.cluster_response_callbacks: | ||
vladvildanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
response = self.cluster_response_callbacks[command]( | ||
response, **kwargs | ||
) | ||
connection._add_to_local_cache(args, response, keys) | ||
return response | ||
except AuthenticationError: | ||
raise | ||
except (ConnectionError, TimeoutError) as e: | ||
|
Uh oh!
There was an error while loading. Please reload this page.