Skip to content

Conversation

@LINKIWI
Copy link
Contributor

@LINKIWI LINKIWI commented Jul 13, 2024

MOVED and ASK responses during command execution in a cluster mode client are not indicative of transient errors for which the client should gracefully delay before a retry. The current behavior unnecessarily constraints throughput in scenarios where:

  • The cluster topology is volatile, e.g. due to active rebalancing (Redis guarantees that keys are moved across nodes atomically during reshard)
  • The hash computation yields an incorrect slot for the command, which is at odds with the cached cluster topology state from the most recent CLUSTER SLOTS load

These effects are particularly pronounced at scale, when MinRetryBackoff is high.

This PR proposes exemption of these classes of errors during the MaxRedirects loop.

Consider the following example benchmark, which uses all default configuration options:

func BenchmarkClient(b *testing.B) {
	ctx := context.Background()
	opts := &redis.ClusterOptions{
		Addrs: []string{"localhost:7000"},
	}
	rdb := redis.NewClusterClient(opts)

	for i := 0; i < b.N; i++ {
		key := "key:" + strconv.Itoa(i)
		args := []interface{}{
			[]byte("GET"),
			[]byte(key),
		}

		err := rdb.Do(ctx, args...).Err()
		if err != nil && !errors.Is(err, redis.Nil) {
			b.Errorf(err.Error())
		}
	}
}

Current behavior, without this patch:

$ go test -bench=. -benchtime=30s ./benchmark/
goos: linux
goarch: amd64
pkg: github.com/redis/go-redis/v9/benchmark
cpu: AMD Ryzen 9 7950X 16-Core Processor            
BenchmarkClient-32    	    2787	  12479787 ns/op
PASS
ok  	github.com/redis/go-redis/v9/benchmark	36.096s

With this patch:

$ go test -bench=. -benchtime=30s ./benchmark/
goos: linux
goarch: amd64
pkg: github.com/redis/go-redis/v9/benchmark
cpu: AMD Ryzen 9 7950X 16-Core Processor            
BenchmarkClient-32    	  953277	     34109 ns/op
PASS
ok  	github.com/redis/go-redis/v9/benchmark	32.902s

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

Successfully merging this pull request may close these issues.

2 participants