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

Handling Redis Cluster configured with cluster-preferred-endpoint-type hostname #1184

Closed
slai11 opened this issue Mar 8, 2023 · 2 comments
Closed

Comments

@slai11
Copy link

slai11 commented Mar 8, 2023

In Redis v7.0, clusterbus extension and hostname support was added which (among other things) adds hostname information into CLUSTER NODES and CLUSTER SLOTS response.

Key changes:

Effects on redis-rb 4.x

Assuming we connect to a Redis Cluster on v7.0 with cluster-preferred-endpoint-type set to hostname.

The Redis::Cluster object will have node_flags made up of ip -> role whereas available_slots made up of hostname -> slots.

The @map in Slot object will be configured with the wrong master nodes since every key lookup in @node_flags will return nil.

With wrongly configured master nodes for a substantial % of the slots, subsequent Redis commands will receive MOVED redirections. This affects performance and possible raise false positives for error metrics on the client side.


At GitLab, we are using redis-rb v4.8.0 and encountered a steady rate of MOVED redirections as a result. We have monkey-patched it via https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112691 in the meantime.

I understand that the project is not testing against Redis v7.0 in the CI (looking at the actions), but would you be open to a PR to upstream our patch? We would be more than happy to prepare a PR.

@byroot
Copy link
Collaborator

byroot commented Mar 8, 2023

I understand that the project is not testing against Redis v7.0 in the CI (looking at the actions)

I'm not sure what gave you this impression:

REDIS_BRANCH: "7.0"

but would you be open to a PR to upstream our patch?

For the current main branch yes, but I'm not interested in a backport for 4.x.

@slai11
Copy link
Author

slai11 commented Mar 8, 2023

Ah apologies. I missed out Cluster when checking https://github.com/redis/redis-rb/actions/runs/4142117293.

For the current main branch yes, but I'm not interested in a backport for 4.x.

That sounds fair 👍 . For the main branch, I've opened a PR in redis-rb/redis-cluster-client@f173f91. It's merged and looks like it will be released in 0.4.3.

In that case, I shall close the issue. Thank you!

@slai11 slai11 closed this as completed Mar 8, 2023
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

No branches or pull requests

2 participants