-
Notifications
You must be signed in to change notification settings - Fork 78
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
Redis: Fix invalid cluster client initialization option #1252
Redis: Fix invalid cluster client initialization option #1252
Conversation
[CHATOPS:HELP] ChatOps commands.
|
Codecov Report
@@ Coverage Diff @@
## master #1252 +/- ##
==========================================
- Coverage 17.98% 17.54% -0.45%
==========================================
Files 501 498 -3
Lines 31465 30966 -499
==========================================
- Hits 5660 5433 -227
+ Misses 25511 25256 -255
+ Partials 294 277 -17
Continue to review full report at Codecov.
|
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.
LGTM
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.
THX :)
Description:
In the current code, on Redis cluster client initialization,
vald/internal/db/kvs/redis/redis.go
Lines 231 to 239 in 53de600
the
opt
argument of theNewClient
option is ignored whenrc.newClient(ctx)
returns no error. However, it is important to useopt
for client initialization becauseopt.Addr
is passed when a new clusterNode instance is created.https://github.com/go-redis/redis/blob/f83600d1a5ac092f27dc672b9dda7cf7adde8bc1/cluster.go#L176-L189
So here, remove the invalid initialization code to fix the problem that occurs when Redis cluster is used.
Related Issue:
Nothing
How Has This Been Tested?:
Local cluster.
Environment:
Types of changes:
Changes to Core Features:
Checklist: