Skip to content
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

A patch makes twemproxy work with redis-sentinel #297

Open
andyqzb opened this issue Dec 17, 2014 · 13 comments
Open

A patch makes twemproxy work with redis-sentinel #297

andyqzb opened this issue Dec 17, 2014 · 13 comments

Comments

@andyqzb
Copy link
Contributor

andyqzb commented Dec 17, 2014

I’m doing a patch for twemproxy. It makes twemproxy work with redis-sentinel to auto detect redis instance failover, and change its forward address.

The design of the patch is shown below.

  1. configure sentinels in configuration like servers.

    Sentinel address can be configured one or more like servers. Twemproxy pick one, and connect.

  2. fetch redis addresses and maintain it consistency with sentinel

twemproxy will send info sentinel and subscribe requests to sentinel when it connects to sentinel.
twemproxy update servers’s addresses from info sentinel response, and fetch redis failover event from subscribe channel. So twemproxy can maintain the consistency of redis addresses with redis-sentinel.
3. sentinel reconnect

Twemproxy will pick an sentinel to reconnect When sentinel connection is done.
A different sentinel will be picked if multiple sentinels are configured. So twemproxy can switch to a good sentinel when some sentinels are done.
Twemproxy will send info sentinel and subscribe requests to sentinel when the new connection is established, just like it connect to sentinel at the first time.
4. Identify the same redis between proxy and sentinel

We configure the servername in the proxy as same as the master-name configured in sentinel. The servername is the redis identification between proxy and sentinel.
For example, configuration in proxy:

servers:
 - 127.0.0.1:6379:1 server1

configuration in sentinel:

sentinel monitor server1 127.0.0.1 6379 2
  1. some small feature

    Twemproxy will dump a new configuration when it changes the redis address. It can let user know the status in the proxy and avoid some problem when proxy is restarted.

That's design of the patch. I’m glad to hear your advices about the patch.

@idning
Copy link
Contributor

idning commented Dec 17, 2014

Great! some thoughts:

  • what's the config of twemproxy looks like?
  • maybe we do not need the servers config?
  • what's the restrictions?

Wish to see this feature in twemproxy.

@rhoml
Copy link
Contributor

rhoml commented Dec 17, 2014

Actually this can be done using sentinel re-configure script, ensuring that the failover is done correctly before modifying twemproxy's yml.

@andyqzb
Copy link
Contributor Author

andyqzb commented Dec 18, 2014

@idning the config with sentinel like the example configuration shown below:

alpha:  
  listen: 127.0.0.1:22121
  hash: fnv1a_64
  distribution: ketama
  auto_eject_hosts: true
  redis: true  
  server_retry_timeout: 2000  
  server_failure_limit: 1  
  servers:  
    - 127.0.0.1:6379:1 server1
    - 127.0.0.1:6380:1 server2
  sentinels:
    - 127.0.0.1:26379:1
    - 127.0.0.1:26380:1

For the second question, it's feasible for the patch. But we have some details to process when the proxy start without servers. For example, twemproxy can't serve client before it fetchs the info from sentinel. further more, It maybe cause proxy start failed when sentinel have problem. So I don't plan to add this feature to the patch now.

The third question, I haven't found restriction yet.

@andyqzb
Copy link
Contributor Author

andyqzb commented Dec 18, 2014

@rhoml Yes, the re-configure script can do the same thing. While script have some shortage compared to the proxy builtin function.

Redis failover won't affect the good servers connection with this patch, while the script will restart the proxy and make all the connection closed.

Builtin function is more reliable than the script. Further more, the script may need to do a remote operation to restart a proxy in another machine, this will increase the risk of failure.

And proxy with the patch can pick a good sentinel when a sentinel is down, while the re-configure script will lose efficacy in this case.

Some other sides, less maintenance cost, failover affect on proxy in read-time, ..

Maybe this patch will spend more time than the script, and it will make the proxy more complicated. But, I think it worth.

@typhoon1986
Copy link

We have written a redis client using sentinel, with these features. Currently we are thinking to put it on twemproxy too. Let me know if you are making some progress, Thank you..

@andyqzb
Copy link
Contributor Author

andyqzb commented Jan 3, 2015

@typhoon1986 I have written 40% code of the patch. I'm too busy to do the patch in the last two weeks. I plan to continue to complete it in the next one or two weeks.

I'd like to know how do you make the client keep in touch with the sentinels.

@typhoon1986
Copy link

@andyqzb very happy to hear that.
well, the client first choose the first available sentinel as the config server, and keeps a connection to fetch redis cluster changes(master down and switch to slave), when this happens, update the inner distribution varibles, so new client call will map to the new server.
Hope this helps.

@kri5
Copy link

kri5 commented Feb 10, 2015

@andyqzb any news?

@andyqzb
Copy link
Contributor Author

andyqzb commented Feb 11, 2015

@kri5 Just remain a reconnect feature for sentinel when conn has some error. I will complete it in this week. You can find the code in my fork project https://github.com/andyqzb/twemproxy/tree/ha_with_sentinel

@mishan
Copy link

mishan commented Feb 19, 2015

I'm excited about this, the prospect of having to restart twemproxy with a NodeJS or Python agent wasn't as exciting.

@andyqzb
Copy link
Contributor Author

andyqzb commented Feb 27, 2015

@mishan the code of the patch is done. I will create a pull request in this week.

@Yancey1989
Copy link

@andypiper Great!Look forward to the patch!

@andyqzb
Copy link
Contributor Author

andyqzb commented Mar 2, 2015

I have created a pull request #324
Hope to hear your feedback for the patch.

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

No branches or pull requests

7 participants