Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Sentinel support #5

Merged
merged 6 commits into from
Aug 31, 2013
Merged

Sentinel support #5

merged 6 commits into from
Aug 31, 2013

Conversation

tovbinm
Copy link
Contributor

@tovbinm tovbinm commented Aug 23, 2013

Add support for Sentinel and use it for automatic master failover for master-slave(s) mode


Done:

  • Add Sentinel monitored Redis client with automatic failover
  • Add support for Sentinel protocol
  • Sentinel tests

TBD:

  • Multi Sentinel support
  • Multi Slave support for reads

- Add Sentinel protocol support
- Some Tests
Conflicts:
	src/main/scala/redis/Redis.scala
@@ -12,3 +12,5 @@ resolvers += "scct-github-repository" at "http://mtkopone.github.com/scct/maven-
addSbtPlugin("reaktor" % "sbt-scct" % "0.2-SNAPSHOT")

addSbtPlugin("com.github.theon" %% "xsbt-coveralls-plugin" % "0.0.3")

addSbtPlugin("com.github.mpeltonen" % "sbt-idea" % "1.5.1")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just add this to your ~/.sbt/plugins.sbt and get it for free across all projects without actually checking it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I work with multiple machines, sometimes I totally forget to update ~/.sbt/plugins.sbt. So I thought it might be handy to have locally in the project.

- Some test modifications
@tovbinm
Copy link
Contributor Author

tovbinm commented Aug 24, 2013

@etaty, I thought about that, aka Decorator pattern. It should be definitely better than calling sc.redisClient().<operation>(...) each time.


override def map(fs: => Fragments) = fs ^ Step(system.shutdown())

def withRedisServer[T](block: Int => T): T = {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could keep withRedisServer around ?
travis-ci.org is slow enough, i don't like to start a cluster if we don't need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll try to fix it.

@tovbinm
Copy link
Contributor Author

tovbinm commented Aug 26, 2013

@etaty @ryanlecompte , please review.

@tovbinm tovbinm closed this Aug 26, 2013
@tovbinm tovbinm reopened this Aug 26, 2013
Do not create a new RedisClient instance each time Sentinel get notified on master server change,
otherwise we'll lose all events in the old client queue.
@tovbinm
Copy link
Contributor Author

tovbinm commented Aug 30, 2013

@etaty , I made some more improvements since the last review. I'd be glad if you could have a look. Thanks.

/cc @ryanlecompte

@ryanlecompte
Copy link
Contributor

Sorry for the delay @tovbinm. The updates look good to me!

@etaty
Copy link
Owner

etaty commented Aug 30, 2013

@tovbinm sorry i will look into it Saturday. (I am not sure about the way you changed the test, i think we can do a setup/cleanup with specs2. Not sure)

@tovbinm
Copy link
Contributor Author

tovbinm commented Aug 30, 2013

Thanks @ryanlecompte, @etaty.

@etaty etaty merged commit 3ebd350 into etaty:master Aug 31, 2013
@etaty
Copy link
Owner

etaty commented Aug 31, 2013

Merged :)

I will edit a little the tests.

@tovbinm
Copy link
Contributor Author

tovbinm commented Aug 31, 2013

Awesome!

@tovbinm
Copy link
Contributor Author

tovbinm commented Aug 31, 2013

We should probably update README as well.

@etaty
Copy link
Owner

etaty commented Sep 1, 2013

Yep, i wait for the release date (end of this week :) ). If you want to do it, you are welcome.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants