-
Notifications
You must be signed in to change notification settings - Fork 704
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
Make cluster replicas return ASK and TRYAGAIN #495
Make cluster replicas return ASK and TRYAGAIN #495
Conversation
After READONLY, a replicas behaves as its primary regarding returning ASK redirects and TRYAGAIN. Without this patch, a client reading from replicas cannot tell if a key doesn't exist or if it's being migrated to another slot. Therefore, without an ASK redirect in this situation, reading from replicas wasn't reliable. The target of a redirect is always a primary. If a client wants to continue reading from a replica after following a redirect, it needs to figure out the replicas of that primary using CLUSTER SHARDS or similar. Signed-off-by: Viktor Söderqvist <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #495 +/- ##
============================================
- Coverage 70.17% 69.81% -0.37%
============================================
Files 109 109
Lines 59904 61802 +1898
============================================
+ Hits 42039 43147 +1108
- Misses 17865 18655 +790
|
@supercaracal reported this for Redis in September 2022:
I think this is what you need. |
Yes, it is. Thank you so much! |
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.
this seems a good idea, LGTM.
Signed-off-by: Viktor Söderqvist <[email protected]>
i guess we also need to update https://valkey.io/commands/readonly/ |
@enjoy-binbin What do you want to write for the READONLY docs? It already says that the replica can return redirects. |
ohh, so in the docs, we already says that the replica can return redirects (before the changes)... |
Yes, but maybe we should add a note like "Before Valkey 8, it was not reliable during slot migrations bla bla bla....." WDYT? |
yean, this is a sometime that we can mention.
btw, in the docs, we says this. This sentence says that "after resharded", the replica can return the redirect, right? and in this PR, replica will return redirect during the slot migrations, right? |
@zuiderkwast ping, i need an ACK in case i misunderstood it |
@enjoy-binbin ACK You're right. We should improve the docs. I think it's almost correct already but we can add point 3.
3. A slot migration is ongoing. In this case the replica can return an ASK redirect or a TRYAGAIN error reply. OK? |
Documentation for valkey-io/valkey#495 --------- Signed-off-by: Viktor Söderqvist <[email protected]>
After READONLY, make a cluster replica behave as its primary regarding returning ASK redirects and TRYAGAIN.
Without this patch, a client reading from a replica cannot tell if a key doesn't exist or if it has already been migrated to another shard as part of an ongoing slot migration. Therefore, without an ASK redirect in this situation, offloading reads to cluster replicas wasn't reliable.
Note: The target of a redirect is always a primary. If a client wants to continue reading from a replica after following a redirect, it needs to figure out the replicas of that new primary using CLUSTER SHARDS or similar.
This is related to #21 and has been made possible by the introduction of Replication of Slot Migration States in #445.
Release notes: