"CROSSSLOT Keys in request don't hash to the same slot" error

[rbroggi]

Is your feature request related to a problem? Please describe

In method getValues in monitor/redis/watcher.go:74 the method MGet cannot be used with an arbitrary set of keys as this leads to an error: CROSSSLOT Keys in request don't hash to the same slot, which means that MGet can only be used for set of keys that hash to the same slot.

Is this a bug? Please provide steps to reproduce, a failing test etc.

Describe the solution

Additional context

[rbroggi]

Simple alternative:

func (w *Watcher) getValues(ctx context.Context, ch chan<- []*change.Change) {
	values := make([]interface{}, len(w.keys))
	for i, key := range w.keys {
		v, err := w.client.Get(ctx, key).Result()
		if err != nil {
			log.Errorf("failed to Get value for key %s: %s", key, err)
			return
		}
		values[i] = v
	}

	changes := make([]*change.Change, 0, len(w.keys))

	for i, key := range w.keys {
		if values[i] == nil {
			continue
		}

		value := values[i].(string)
		hash := w.hash(value)
		if hash == w.hashes[i] {
			continue
		}

		w.versions[i]++
		w.hashes[i] = hash

		changes = append(changes, change.New(config.SourceRedis, key, value, w.versions[i]))
	}

	if len(changes) == 0 {
		return
	}

	ch <- changes
}

[rbroggi]

same code can be done with go-routines

[mantzas]

same code can be done with go-routines

I would suggest to start without goroutines because it might has side effects with a lot of settings, until it becomes a problem of course.

[rbroggi]

alright, no problem for me.

[aziule]

I think we should give the user the possibility to provide a config flag to either use a single instance mode (with MGET) or cluster mode (with GETs).

[mantzas]

That would be a too complex thing and not so nice API wise.
We had to explain when to sue what, etc.

[aziule]

I fully agree that it adds complexity, but multiple GET will have an impact for users with lots of keys

[mantzas]

Not necessarily. We are talking about how fast to update a setting change which might be ok to take some time e.g. 10s of seconds to minute. We are already polling and the interval can be minutes.

[rbroggi]

As an example, our application uses a polling interval of 5mins (quite high) in almost all environments. The only exception is the local environment where we increase the polling frequency by having an interval of 50 milliseconds ( which might be a bit tight to fulfill multiple roundtrips between the service and redis) - the reason we have a high polling interval is to be able to replicate our changes in configuration in our test scenarios in a fast fashion. In any case, I have tested that our local integration tests work even with this high frequency (keep in mind that we only have 3 redis configurations, so 3 roundtrips every 50milliseconds).