Skip to content

Support for SENTINEL commands #370

Merged
merged 1 commit into from Jan 30, 2013

3 participants

@hamsterready

I have added SENTINEL related methods to Jedis class down to Protocol via Client and BinaryClient classes.

Probably instead of having so many new methods one could have SentinelParams class (just like SortingParams for sort).

At the same time was not sure about checkIsInMulti() - should it be in every call now? Most does not apply to SENTINEL methods as MULTI is not allowed.

Test based on config files taken from noise/redis-sentinel-tests.

Time to build hamsterready/jedis-sentinel-pool - still might need to refactor JedisPool as it is not extendable (non empty constructors are evil ;-)).

@jmlogan
jmlogan commented Dec 15, 2012

Do we have a status on this? Supporting Sentinel is important to a lot of users. It would be a pity for Jeids to not support Sentinel when it is (hopefully soon) declared stable.

@hamsterready

There is doc somewhere at redis.io where antirez decribes how clients should behave. Afaik my pull brings some of that behaviour but not everything.

@xetorthio
Owner

Nice pull request! I have reviewed the code and I like it. I'm not sure how you run the tests... do you need a sentinel up&running?

@hamsterready

Hi @xetorthio,

Grab configs from https://github.com/noise/redis-sentinel-tests and fire up master, slave and sentinels as described on that page:

Open a bunch of shells and fire up redis instances:

redis-server redis-master.conf
redis-server redis-slave1.conf

redis-sentinel sent1.conf
redis-sentinel sent2.conf
redis-sentinel sent3.conf

@xetorthio xetorthio merged commit 69b5291 into xetorthio:master Jan 30, 2013
@xetorthio
Owner

Merged!
Just changed the way it is tested to be automatic, so you keep the same simple "make test" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.