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

make Command.create() return existing instance for known commands #413

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Oct 19, 2023

When the Command.create() method is used to create a Command instance for a known command (not recommended, but also not forbidden), it used to return a generic instance that doesn't know anything about where the keys are in the command. Such generic instance is unusable with Redis cluster, because the target node will be selected randomly, not based on the key, and there's high chance such command will result in the MOVED redirect.

With this commit, Command.create() will return a pre-existing static instance for known commands, which is key-aware and works with Redis cluster.

@Ladicek
Copy link
Contributor Author

Ladicek commented Oct 19, 2023

FTR, my main motivation is to fix quarkusio/quarkus#28704, but I assume direct users of Vert.x Redis client can make the same mistake.

When the `Command.create()` method is used to create a `Command` instance
for a known command (not recommended, but also not forbidden), it used to
return a generic instance that doesn't know anything about where the keys
are in the command. Such generic instance is unusable with Redis cluster,
because the target node will be selected randomly, not based on the key,
and there's high chance such command will result in the `MOVED` redirect.

With this commit, `Command.create()` will return a pre-existing static
instance for known commands, which is key-aware and works with Redis cluster.
@vietj vietj merged commit 727eb5a into vert-x3:master Nov 13, 2023
4 checks passed
@vietj vietj added this to the 5.0.0 milestone Nov 13, 2023
@vietj
Copy link
Contributor

vietj commented Nov 13, 2023

thanks @Ladicek

@Ladicek Ladicek deleted the known-commands branch November 13, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants