Core: SlotMap refactor - Added NodesMap, Update the slot map upon MOVED errors #2682
Conversation
barshaul
force-pushed
the
update_slots_on_moved
branch
from
84c4df4 to
c6bf0a2
Compare
barshaul
changed the title
barshaul
force-pushed
the
update_slots_on_moved
branch
from
c6bf0a2 to
ba16669
Compare
|
How can we test this with a real cluster and client? I think we can do E2E tests with |
ATM we don't have a way to test the inner behavior of the cluster client in redis-rs, so we need to use mocks to make sure the right number of errors were caught/commands were executed internally. |
Signed-off-by: barshaul <[email protected]>
Signed-off-by: barshaul <[email protected]>
barshaul
force-pushed
the
update_slots_on_moved
branch
from
5b40be7 to
8dff69c
Compare
| let mut conn = connections.remove(addr).unwrap(); | ||
| let addr = addr.to_string(); | ||
| if connections.contains_key(&addr) { | ||
| let mut conn = connections.remove(&addr).unwrap(); |
There was a problem hiding this comment.
Better handling here: no unwrap()
There was a problem hiding this comment.
it's the non async cluster implementation, which we don't use. We should eventually remove this code from our repo.
Signed-off-by: barshaul <[email protected]>
barshaul
force-pushed
the
update_slots_on_moved
branch
from
8dff69c to
68e7fa4
Compare
Description
This PR was previously reviewed in our redis-rs fork at amazon-contributing/redis-rs#190 but was on hold due to lock issues. The recently merged PR #2643 addresses the encountered lock issues, making this PR now ready for merging.
This pull request introduces two significant updates:
SlotMap Refactor: The SlotMap structure has been updated with the addition of a
NodesMap, which allows shard addresses to be shared between shard nodes and slot map values. This refactor optimizes how shard information is managed. For a detailed explanation, refer to PR #185.The diagram on the left illustrates the current SlotMap design, while the diagram on the right shows the newly implemented structure:
MOVED Error Handling: Logic has been added to update the slot map in response to
MOVEDerrors. Previously, this was handled only during refresh_slots operations. With this change, the slot map is updated immediately upon encountering aMOVEDerror. More information can be found in PR #186.Issue link
This Pull Request is linked to issue (URL): closes #2123
Checklist
Before submitting the PR make sure the following are checked: