* mget leads to crossslot keys don't hash to the same slot #110
Conversation
monitor/redis/watcher.go
Outdated
| if err != nil { | ||
| log.Errorf("failed to MGET keys %v: %v", w.keys, err) | ||
| return | ||
| values := make([]interface{}, len(w.keys)) |
There was a problem hiding this comment.
We should be safe to use []string here because Get(...).Result() returns a string. It would reduce the complexity and prevent from casting to interface{} and then again to string
There was a problem hiding this comment.
at most []*string because we use nil to signal a bad retrieval
| func (w *Watcher) getValues(ctx context.Context, ch chan<- []*change.Change) { | ||
| values, err := w.client.MGet(ctx, w.keys...).Result() |
There was a problem hiding this comment.
deleted keys would get nil values as you can see from the lib unit tests:
https://github.com/go-redis/redis/blob/master/commands_test.go#L1165-L1173
It means that deletion operations would not be pickedup by the service.
monitor/redis/watcher.go
Outdated
| } | ||
| // notice: if the key is not found we default to the same behavior as if | ||
| // the key was found with an empty value string | ||
| if strCmd.Err() != nil && strCmd.Err() != redis.Nil { |
There was a problem hiding this comment.
we can now trigger update to "" in case the key was deleted generating consistency across pods
rbroggi
force-pushed
the
redis-mget-error-crossslot
branch
from
f2a0c4b to
65363c0
Compare
rbroggi
requested review from
c0nstantx,
Lazaros-Tsigkakos and
mantzas
as code owners
monitor/redis/watcher.go
Outdated
| @@ -70,20 +70,35 @@ func (w *Watcher) monitor(ctx context.Context, ch chan<- []*change.Change) { | |||
| } | |||
| } | |||
|
|
|||
| func toPtr(s string) *string { | |||
There was a problem hiding this comment.
can we inline this? it is only used in one place.
Which problem is this PR solving?
#111
#113
Short description of the changes