Improvement: Implement redis READONLY feature for slaves

[allanwax]

Redis (cluster?) currently allows you to issue a READONLY command on a slave that will allow you to read keys that normally would issue a MOVED response. In cases where you can tolerate the slave not being entirely up to date with the master, it would be nice if there is a way to do something like hand waving here find a slave for a key resident on a master, (b) connect to the slave in READONLY mode, (c) read key:values. This would help with throughput in certain cases.

[HeartSaVioR]

Great point!
Btw, we have some restrictions for handling slave in nature of Redis.

1. Redis Sentinel
   There're many requests to support slave with query offloading.
   But Redis Sentinel doesn't failover slave, and doesn't determine ODOWN.
   So connection to slaves could be broken when we need it.
   We need some strategies to maintain good slaves.
2. Redis Cluster
   I can't remember Redis Cluster could take care of slaves.
   (I was seen related content from Redis Cluster Specification doc. but I can't remember. ;( )

So when we add READONLY feature, user should maintain good slaves and pick one, and send commands.
What do you think about? Is it fine, though?

[allanwax]

Some background. Before we used the cluster, the arrangement was a master and at least one slave. Writes only went to the master and reads only went to the slave(s). Of course the master was keeping the slave(s) updated. This way we could optimize writing the data and more importantly, reading the data. A write would never interfere (atomic operation) with the read, at least conceptually.

[HeartSaVioR]

@allanwax
I've skimmed Redis Cluster Specification, and it has same restriction from Redis Sentinel.
PFAIL is SDOWN, FAIL is ODOWN, and slave doesn't failed over.

Query offloading is definitely possible for Redis and Redis Cluster. Redis Cluster Specification describes query offloading.
All thoughts about slave is "how we can maintain good slaves to query?".

[allanwax]

I'm kind-of ignoring "how we can maintain good slaves to query?" for now, not that this is a trivial question. I think we already do a good job of maintaining good slaves according to my understanding of the phrase and intended meaning. My original question was allowing the READONLY command (or equivalent) to be used with the understanding that you take the risks associated with that command. I hope we're talking about the same thing or the same related set of things.

[HeartSaVioR]

@allanwax OK, I see your intention about this issue.
That would be OK for normal Redis M/S relation.
We would like to add READONLY command to BinaryJedis. Thanks for finding missed spot!

Just leaving additional note about READONLY with Cluster...
We can't handle READONLY to cluster without maintaining slaves, because only JedisCluster knows cluster's nodes and slot-"master node" relations - though some of these are not matched to current - and doesn't care about slaves for now.
Master doesn't let us move whether we set READONLY or not, so we have to maintain slot-slave relations.

tl;dr. We're fine with READONLY with Jedis, but not ready for JedisCluster.

[allanwax]

To be clear the intent was to send a READONLY to one or more of the slaves. The assumption here is that putting a slave in READONLY does not stop updates from the master. Part of my original comment about being able to get a hold of a slave knowing a key, or by being able to enumerate the slaves and pick one, was to enable getting a hold of a single slave. My intent is not to make the whole cluster READONLY, just (a) slave(s).

Again, based on the above it would make sense to surface the method(s) that enumerate the master and slaves in such a way that it would be simple to select a jedis instance, put it into READONLY mode and use it. Also when you discontinue use in your thread of that particular host, it would fall back to being in a normal state.

I don't know if putting a particular redis instance into READONLY puts it into that state for all users of that instance. If it does, that's an issue to be addressed.

[HeartSaVioR]

We should think about slot migration.
With this constraints, holding a slave doesn't work.
We could more safe with using "cluster info" just before getting a slave and send command immediately, but it doesn't prevent slot migration between executing commands.
In addition, if we expose Jedis instance of one of cluster nodes, users may need to know MOVED, and ASK, TRYAGAIN, etc which is hiding from JedisCluster.

[allanwax]

I agree with the potential problems. However, part of what I would like to document is that is is dangerous to use this command. Also, I don't know if replication stops if you use it.

Maybe we need a jedis specific command that allows someone to get something from a slave and ignore the MOVED message. This wouldn't break what's already there. But writes would have to be prohibited.

[HeartSaVioR]

One thing I'm being afraid of is, many users doesn't read documentations, and post an issue that why this is not working.
This means, "AT YOUR OWN RISK" may not work.

@xetorthio @marcosnils I'd like you to discuss this issue when you have time. :)

Btw, maybe we can find a way to handle it inside JedisCluster, with minimizing potential problems.

There may be another issue, clearing READONLY state when returning instance to pool if we borrowed a instance.

[xetorthio]

I think that there are 2 things here:

1. Supporting the READONLY AND READWRITE commands
2. Decide if something needs to be done to JedisCluster

I think 1 is a no brainer and easy to do.
Regard 2. I think nothing should be done. The current behavior of
JedisCluster should support this.

Am I missing something?

On 21:16, Mon, Nov 24, 2014 Jungtaek Lim notifications@github.com wrote:

One thing I'm being afraid of is, many users doesn't read documentations,
and post an issue that why this is not working.
This means, "AT YOUR OWN RISK" may not work.

@xetorthio https://github.com/xetorthio @marcosnils
https://github.com/marcosnils I'd like you to discuss this issue when
you have time. :)

Btw, maybe we can find a way to handle it inside JedisCluster, with
minimizing potential problems.

There may be another issue, clearing READONLY state when returning
instance to pool if we borrowed a instance.

—
Reply to this email directly or view it on GitHub
#790 (comment).

[HeartSaVioR]

@xetorthio
Unfortunately JedisCluster's slot-node cache cares about master nodes, not slave nodes.
Sometimes slot-node cache update is only based on MOVED response, which only points to master node.
And we can not ensure that slave node serves requested keys "now" because key can move between retrieving slot node and sending command.

As I talked before from Google Groups (https://groups.google.com/d/msg/jedis_redis/u6j8slokO3E/Dh5Q94TRjJUJ), it's not that simple.
If we delegate handling additional things (MOVED, ASK, etc) to users, then it could be simple.
But I'm wondering it's acceptable by users.

If we add new feature to

1. take care of slaves
2. send READONLY & command with handling re-route (definitely need it)
3. discard connections which are used to run operation (because of READONLY state), or reset state to READWRITE
   I think we would be fine. :)

Please correct me if I'm wrong. Thanks!

[allanwax]

My intent is not to handle MOVED at all. My needs are less restrictive. If I put a slave in READONLY mode then I am willing to accept the 'loss' of keys without handling the moved responses. I believe that a MOVED is not issued in READONLY mode which is fine for me. The intent is to have writes to the master and reads from the slave. Periodically, rechecks will be made as to where to read from. It is also assumed that the master is updating the slave, wherever the key lies, regardless of READONLY or not.

With this the ability to find where a key exists needs to be surfaced as well. Something like Jedis jedis = getSlave(key);

[HeartSaVioR]

"NEED CITATION"

I just read Redis source code, and READONLY keyword works only in cluster environment.

Then we have to manage slaves to serve READONLY feature.
Note that Redis cluster redirects us to master node if we send command to node which is not holding a key. (Query-offload doesn't work, either.)
So keeping fresh slot-slave cache is most important part of READONLY feature.

[allanwax]

SEE #830

[franklinjohnv-flipkart]

Hi Allan/Heart,
After going through thread, I'm having some confusions. Can you please explain the same?
a) Traditional Redis - Master and Slaves monitored by a sentinel. JedisSentinelPool gives the current master to the client code, and can be used to set/get values. Does Sentinels provide info about slaves, and can we make use of the same info to load balance GET requests by sending them to slaves if absolute consistency is not required? (Current Jedis doesn't support this?)
b) Redis Cluster - In alpha mode now, where multiple masters (with their slaves) hold their respective shards of the data. Automated reassignment of shards are ensured (split brain cases or capacity addition). Requests for keys to a master processed as a) If belonging to selected master's shard - executed by master itself and b) If any other shard - request forwarded to the respective master (or client is intelligible enough to contact the correct master with some information provided by sentinel nodes ?)

[allanwax]

I will leave this to @heart to respond. I think he knows more about this than I.

I can say that Jedis takes care of handling MOVED messages and directing requests to the proper place.

[HeartSaVioR]

a) JedisSentinelPool doesn't care about slaves, cause Redis Sentinel doesn't take care of slave odown / failover.
b) AFAIK there's no automated slot reassignment in Redis Cluster. Only master/slave failover supported.

[franklinjohnv-flipkart]

Thanks Allan and Heart..

[thiyaml]

Wondering to know if this is still not supported where sentinel pool can be used to talk to slaves for read so that load is distributed.

And @franklinjohnv-flipkart how you managed for (a) to load balanced slaves?

[punkeel]

+1 on this one, even if I know it's hard to implement properly ... :/

[github-actions]

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