Real Redis cluster

[dmytroDragan]

add support of Real Redis Cluster which is compatible with JedisCluster

[kstyrc]

Hi Dragan,

thanks for the contribution. As far as I can see, Redis 3.0 cluster is smth different than ephemeral/sentinels in previous Redis releases. That's pretty unfortunate, as we already have the name RedisCluster in the project... https://github.com/kstyrc/embedded-redis/blob/master/src/main/java/redis/embedded/RedisCluster.java For now I have to idea how to integrate these both concepts and have meaningful namespace.

Additionally, we now need jedis jar as dependency to create a cluster and check its setup success/failure. That seems fine. Maybe I should also use jedis to check that a single node redis is up&running instead of pattern-matching std output?

Would it be possible for Cluster to implement/extend Redis interface? Does it make sense?

I'll comment more later regarding code review. I believe there is a couple of places to refactor/improve.

[dmytroDragan]

Hi Krzysztof,

1) integrate these both concepts

As far as I see, for now, it is absolutely different approaches which we can not combine (for example, we can not setting up slaveof property if want enable cluster mode and etc)

2) Additionally, we now need jedis jar as dependency to create a cluster and check its setup success/failure. That seems fine. Maybe I should also use jedis to check that a single node redis is up&running instead of pattern-matching std output?

I agree.

3)Would it be possible for Cluster to implement/extend Redis interface? Does it make sense?

I think, yes (we can combine start and create operations or extend Cluster interface with Redis).

4)I believe there is a couple of places to refactor/improve.

Tried to do it asap, so I'm opened for discussion.

Best regards, Dmytro Dragan On Jul 20, 2015 23:08, "Krzysztof Styrc" notifications@github.com wrote:

Hi Dragan,

thanks for the contribution. As far as I can see, Redis 3.0 cluster is smth different than ephemeral/sentinels in previous Redis releases. That's pretty unfortunate, as we already have the name RedisCluster in the project... https://github.com/kstyrc/embedded-redis/blob/master/src/main/java/redis/embedded/RedisCluster.java For now I have to idea how to integrate these both concepts and have meaningful namespace.

Additionally, we now need jedis jar as dependency to create a cluster and check its setup success/failure. That seems fine. Maybe I should also use jedis to check that a single node redis is up&running instead of pattern-matching std output?

Would it be possible for Cluster to implement/extend Redis interface? Does it make sense?

I'll comment more later regarding code review. I believe there is a couple of places to refactor/improve.

— Reply to this email directly or view it on GitHub https://github.com/kstyrc/embedded-redis/pull/57#issuecomment-123013890.

[kstyrc]

Made some comments. In general, the most important points from my perspective:

1) Naming: I am strongly convinced it should be named RedisCluster, but then how should we name the config with sentinels?

2) Subpackage: My suggestion is to move your impls into redis.embedded.cluster subpackage and extract any relevant static classes if necessary.

3) Jedis dependency: Is there any way to setup Redis Cluster without adding Jedis dependency?

4) Examples of usage: It would be of great help, if you could update README with usage of Redis Cluster as soon as we'll finish with the changes

5) Unit tests: how can we verify that Redis Cluster has been setup correctly?

As soon as we'll merge this PR. I'll make a follow-up PR with naming changes.

[dmytroDragan]

1) see my answer in https://github.com/kstyrc/embedded-redis/pull/57#discussion-diff-35511889 2) ok 3) Yes, theoretically we could. Official Redis documentation use rb-client. So we can run it as process, but I think we can have better control with Jedis. 4) Sure, after we come to production stage. 5) By receiving "OK" state from Redis Cluster. Please look at http://redis.io/commands/cluster-info

[krishna81m]

any update on this merge and https://github.com/fmonniot/embedded-redis?

[dmytroDragan]

I'm not sure if it is still valid, so I will close it.