Skip to content
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

add support for read_from_replicas in a none read_only mode #1878

Closed
fantamiracle opened this issue Jan 12, 2022 · 6 comments
Closed

add support for read_from_replicas in a none read_only mode #1878

fantamiracle opened this issue Jan 12, 2022 · 6 comments
Labels

Comments

@fantamiracle
Copy link

fantamiracle commented Jan 12, 2022

For RedisCluster, are there plans to have a separate flag for read_only to support read_from_replicas flag for when read_only is disabled?

@fantamiracle fantamiracle changed the title read_from_replicas in a non read_only mode read_from_replicas in a none read_only mode Jan 12, 2022
@fantamiracle fantamiracle changed the title read_from_replicas in a none read_only mode add support for read_from_replicas in a none read_only mode Jan 12, 2022
@barshaul
Copy link
Contributor

@fantamiracle, I'm not sure I'm following, can you elaborate?
What does 'read_from_replicas' mean when read_only is disabled? If READONLY is not set, how can we read from replicas?

@rtkefreure
Copy link

It sounds kind of like something that would allow arbitrary Redis commands to read from replicas. Is that the case @fantamiracle? A feature like that might be useful to allow commands like EVAL and EVALSHA to read from replicas when the scripts do read only operations. I put up an issue for EVAL and EVALSHA here Grokzen/redis-py-cluster#480 on the redis-py-cluster repo.

@chayim chayim assigned dvora-h and unassigned dvora-h Jan 26, 2022
@barshaul
Copy link
Contributor

barshaul commented Jan 30, 2022

Hello @rtkefreure, EVAL/EVALSH commands have not yet been implemented in RedisCluster API because we are unable to parse the passed script to determine which node we need to run it against, if all keys belong to the same node/slot, and other cluster limitations.
A user who knows what he's doing and uses a cluster-safe script where all keys belong to the same slot can obtain the Redis client instance of a particular node and run the EVAL against it using the Redis class API. This can also be done with a replica node.
For example:

from redis import RedisCluster
r = RedisCluster('localhost', 16379, read_from_replicas=True)
r.set('{foo}1', 'bar1')
r.set('{foo}2', 'bar2')
replica_node = r.get_redis_connection(r.get_node_from_key('foo', replica=True))
script = 'return redis.call("MGET", "{foo}1", "{foo}2")'
print(replica_node.eval(script, 0))

// output: [b'bar1', b'bar2']

However, you do need to enable readonly mode in order to run this script (see read_from_replicas=True in the RedisCluster initialization). So i'm still not clear about what you mean by reading from replicas when readonly mode is disabled.

@rtkefreure
Copy link

rtkefreure commented Jan 31, 2022

Hi @barshaul, thanks for the information. When you say that EVAL/EVALSHA are not yet implemented in the RedisCluster API, is that a change from redis-py-cluster? We are using EVAL commands with redis-py-cluster right now and it seems to be working fine (aside from not being able to force a script to run on a replica that we know will not do write operations). It looks like the code that determines the node/slot for the keys is here https://github.com/Grokzen/redis-py-cluster/blob/f0627c91ce23e8784dbc996078428c9bdbacb20b/rediscluster/client.py#L484-L490

For example, this seems to work fine for with redis-py-cluster:

from rediscluster import RedisCluster

startup_node = {
    'host': '127.0.0.1',
    'port': '1234'
}
redis_client = RedisCluster(startup_nodes=[startup_node],
                                         decode_responses=True,
                                          skip_full_coverage_check=True,
                                          read_from_replicas=True)

write_script = "redis.call('SET', KEYS[1], ARGV[1], 'EX', ARGV[2])"
redis_client.eval(write_script, 1, "the_key", "the_value", 600000)
read_script = "return redis.call('GET', KEYS[1])"
print(redis_client.eval(read_script, 1, "the_key"))

Will this not work in redis-py with the newly added cluster support?
Thanks!

@rtkefreure
Copy link

It looks like something like this might allow eval to work with the redis-py cluster implementation:

from redis.cluster import RedisCluster
from redis.cluster import ClusterNode


class RedisClusterWithEval(RedisCluster):
    def _get_command_keys(self, *args):
        """
        Get the keys in the command. If the command has no keys in in, None is
        returned.
        """
        if args[0].lower() == "eval":
            numkeys = args[2]
            keys = args[3: 3 + numkeys]
            return keys
        else:
            return super()._get_command_keys(*args)

    def eval(self, script, numkeys, *keys_and_args):
        return self.execute_command("EVAL", script, numkeys, *keys_and_args)


nodes = [ClusterNode('127.0.0.1', 30001)]
rc = RedisClusterWithEval(startup_nodes=nodes)

write_script = "redis.call('SET', KEYS[1], ARGV[1], 'EX', ARGV[2])"
rc.eval(write_script, 1, "the_key", "the_value", 600000)
read_script = "return redis.call('GET', KEYS[1])"
print(rc.eval(read_script, 1, "the_key"))
print(rc.get("the_key"))

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2023

This issue is marked stale. It will be closed in 30 days if it is not updated.

@github-actions github-actions bot added the Stale label Feb 2, 2023
@github-actions github-actions bot closed this as completed Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants