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

Update slotmap when slot is not served by any node #192

Merged
merged 8 commits into from Sep 29, 2023

Conversation

bjosv
Copy link
Collaborator

@bjosv bjosv commented Sep 27, 2023

The sync-API will now update the slotmap before attempting to send a command when the slot is not served.

The async-API will initate the slotmap update in parallell so that the next command might have an updated slotmap.

This also fix an additional issue by using the correct function name for redisClusterUpdateSlotmap().
The former name cluster_update_route() is not defined when HIRCLUSTER_NO_OLD_NAMES is defined, which would break the build.

Fixes #191

The sync-API will update the slotmap before attempting to send a command
when the slot is not served by any cluster node.

The async-API will initate the slotmap update in parallell so that the next
command might have an updated slotmap.
Its former name `cluster_update_route()` can be removed by the
define HIRCLUSTER_NO_OLD_NAMES, which would break the build.
Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

Only some questions about the other (slightly unrelated) fixes.

tests/clusterclient_async.c Show resolved Hide resolved
tests/ct_connection.c Outdated Show resolved Hide resolved
tests/ct_connection.c Outdated Show resolved Hide resolved
tests/scripts/slots-not-served-test-async.sh Outdated Show resolved Hide resolved
@bjosv
Copy link
Collaborator Author

bjosv commented Sep 29, 2023

I reverted the callback handling for AUTH commands, to be added via separate PR.
There are other authentication related changes that can be discussed in that PR.

Also removed the use of the eventloop in the testcases that didn't need any events for its testing.

Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Let's take the other things in other PRs.

@bjosv bjosv merged commit 0196253 into Nordix:master Sep 29, 2023
32 checks passed
@bjosv bjosv deleted the handle-slots-not-served branch September 29, 2023 08:57
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 this pull request may close these issues.

Recovery in case of discovered slots from redis cluster is partial
2 participants