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

Sentinel support #43

Closed
benbro opened this issue Aug 22, 2021 · 5 comments · Fixed by #62
Closed

Sentinel support #43

benbro opened this issue Aug 22, 2021 · 5 comments · Fixed by #62

Comments

@benbro
Copy link

benbro commented Aug 22, 2021

Are there plans to add Sentinel support for high availability?
Initial implementation in this issue.

@zuiderkwast
Copy link
Collaborator

Sentinel support would be nice! Can you summarize what a client needs to do to support Sentinel?

We should also have eredis_cluster in mind. Is there anything special to thing about for sentinel + cluster?

@benbro
Copy link
Author

benbro commented Aug 23, 2021

Guidelines for Redis clients with support for Redis Sentinel
The client use sentinel nodes to find the master on first connection and reconnection.
Redis cluster and sentinel have different use cases.
Redis cluster provides sharding and high availability. It requires minimum 6 machines - 3 masters and 3 replicas.
Sentinel provides only high availability. It requires 3 sentinel nodes (for quorum), 1 master node and at least 1 replica. It's possible to have 3 machines each with redis node and sentinel node.

@zuiderkwast
Copy link
Collaborator

Ah, so if you use cluster, you don't need sentinel? Good to know. I thought it's possible to combine both.

Thanks for the link!

We don't have plans to implement sentinel support (because we use cluster) but if you implement it, we will accept a PR.

Regarding how it will affect eredis, the old PR added

To enable sentinel support for eredis app:

Start eredis_sentinel main process under supervisor with list of all sentinels as argument:

eredis_sentinel:start_link([{"host1.lan", 20367}, {"host2.lan", 20367}]).

When starting eredis clients use string sentinel:master_name instead host:

eredis:start_link("sentinel:mymaster", 0).

I think this way of starting eredis seems a bit strange. Can we instead have {sentinel, SentinelOpts} as one of the options to eredis:start_link/1? The eredis_sentinel can be a helper module but we want the public API in the eredis module.

If the sentinel code is expected to be large, we could also consider having it as a separate repo, if that's possible, just like we have eredis_cluster as a separate repo with dependency to eredis. Then we would just add minimum code to eredis so that it can refer to eredis_sentinel.

WDYT?

@benbro
Copy link
Author

benbro commented Sep 18, 2021

Yes wen can have {sentinel, SentinelOpts} and eredis_sentinel module.

  1. If the sentinel option is set, eredis_client will need to get the master host from eredis_sentinel:get_host(SentinelOpts) and only try to connect when it get it.
  2. eredis_sentinel will find the master host from one of the sentinel nodes.
  3. eredis_client will remove the host from the state on disconnection and get a new one from eredis_sentinel in case the master changed.

@zuiderkwast
Copy link
Collaborator

Looks good.

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

Successfully merging a pull request may close this issue.

2 participants