-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Redis connection health checks and restarts #55
Add Redis connection health checks and restarts #55
Conversation
@jwoertink is ready for initial impressions review. I still have to add specs. I will sort that out tomorrow. |
Thanks @mjeffrey18! @russ and @franciscoGPS if either of you have time, I'd love more eyes on this. |
a4b7828
to
b6994f0
Compare
This looks awesome. Can’t wait to try it out. |
This PR plus - jgaskins/redis#35 is what we are running in production |
b6994f0
to
5150649
Compare
@jwoertink Added specs for this also. We can merge if you guys are ready. |
@@ -197,6 +199,64 @@ class ChatChannel < ApplicationCable::Channel | |||
end | |||
``` | |||
|
|||
## Redis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding this section on README is amazing, thanks for such effort 😎 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet! Thanks for doing this. Sorry for the delay, I was at Euruko last week. Just got back and catching up.
Redis has complexities that need to be considered;
This PR helps to mitigate issues 1 and 2
We've been running these changes in production (with these changes #54 and a different Redis shard) with excellent results;
1 month of APM metrics
Workloads
2 Servers will low resource usage