-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
FailoverClient with read-only support #1199
Conversation
|
Good point @dstrop I'll fix that |
Hi people, is there something that I can do to help you with it? I'm so interested in this feature. |
I want this feature too! When will the commit be merged? |
should we listen the sentinel's |
is there any chances for this PR to come up in v8 somehow ? |
there is a conflicting file |
7b93c9c
to
75cfccc
Compare
75cfccc
to
f1ba4e5
Compare
@vmihailenco , could you review this PR? |
@nextsux sorry for ignoring this. The code looks clean and self-contained so I don't mind merging it. But I guess it will not "properly" balance the load for 2 reasons:
But that is all theory and in practise this PR may be good enough for some workloads. @nextsux do you have any experience using this PR under the load? First In, First Out queue was picked so some connections are unused at all and we can close them after some time. I think it is not terribly useful and we can change pool to use other strategy if it helps with this PR. But really using ClusterClient (PR #997) looks more appealing to me. It already has all the logic to properly balance the load - all we need is some flag / refactoring to disable cluster specific features... |
no problem @vmihailenco. red one is current master which is not receiving read traffic. |
Good, let's hope it will work even better for others too. Meging. |
Allow
FailoverClient
ReadOnly
option. Client then connects to redis replicas (or master if no slave is available) for performing read-only queries. User is responsible for deciding which query can be run against it.It aims to implement the same as #997 but without misusing ClusterClient which is not suitable for this.