search

lucee / extension-redis

Lucee Redis Cache Extension
11 stars 13 forks source link

Add Redis Cluster Support #9

Open andrew-dixon opened 3 years ago

andrew-dixon commented 3 years ago

We have set up and configured a Redis cluster that is working, however, the Lucee Redis extension does not appear to support the "MOVED" response and is instead returning null. As a simple example, using the redis-cli:

[root]# redis-cli -h localhost -p 6379 -c set foo bar
OK
[root]# redis-cli -h localhost -p 6379 get foo
"bar"
[root]# redis-cli -h localhost -p 6382 get foo
(error) MOVED 12182 localhost:6379

If we then add the -c option (cluster mode) we get:

[root]# redis-cli -h localhost -p 6382 -c get foo
"bar"

As expected as the -c option Enable cluster mode (follow -ASK and -MOVED redirections)., however, with Lucee, if I configure two Redis caches, using the above host and ports and do:

<cfscript>
        cacheput('foo', 'bar', createTimespan(0,0,0,30), createTimespan(0,0,0,30), 'test-redis');
        dump(cacheget('foo', false, 'test-redis'));
        dump(cacheget('foo', false, 'test-redis-2'));
</cfscript>

The response I get is:

string  bar
Empty:null

So, clearly, the Lucee Redis extension is not following the "MOVED" response from the cluster and retrieving the key value from the location given.

andrew-dixon commented 3 years ago

Actually, I think there is probably more that needs doing for "real" cluster support. From reading over the spec (https://redis.io/topics/cluster-spec) I think it would need to be able to support the definition of multiple servers in the cache definitions as well as dealing with messaging about cluster node changes, e.g. promotions, demotions, etc... Will change the title of ticket.

tbenton commented 3 years ago

@andrew-dixon We use a sentinel configuration and it seems to work for us. However, it appears 2.9.0.10-RC is the last release that has Redis sentinel support. I am not sure if you looked at that version to see if it will support your needs.

The biggest trick is the host list needs to be separated by Chr(10) not by commas.

andrew-dixon commented 3 years ago

@tbenton My understanding is that Sentinel and Cluster are two different things. In this case the Redis Cluster is being used already, it is just Lucee isn't following the responses to get the result from the cluster as it needs to.

tbenton commented 3 years ago

@andrew-dixon Sentinel and Cluster are two different things. I know in sentinel that driver takes care of where to get the data. I assume the Redis driver would have to figure out which node to get data from.

Sorry I have not done much with Cluser

andrew-dixon commented 3 years ago

@tbenton When you query a cluster node, if the data is on a different node it responses with a message telling you which node it is on, e.g.

(error) MOVED 12182 localhost:6379

It is then up to the client to remake the request to that node. At present, Lucee errors.

tbenton commented 3 years ago

@andrew-dixon Which version of the extension are you using?

andrew-dixon commented 3 years ago

@tbenton I tried up to the latest beta version that was available at the time of raising the ticket.

jlamoree commented 1 month ago

Is there no hope for Redis cluster support in the Lucee Redis Cache Extension? Are Lucee users instructed to buy the Ortus Solutions Redis Cache extension instead?

The project readme indicates that this is based on Jedis, but I think it may have been inspired by Jedis, rather than dependent on Jedis. I assume that is the impediment to supporting Redis clusters -- it would be all new work to implement.