release: 9.7.1 patch (#3278)

* Add guidance on unstable RESP3 support for RediSearch commands to README (#3177) * Add UnstableResp3 to docs * Add RawVal and RawResult to wordlist * Explain more about SetVal * Add UnstableResp to wordlist * Eliminate redundant dial mutex causing unbounded connection queue contention (#3088) * Eliminate redundant dial mutex causing unbounded connection queue contention * Dialer connection timeouts unit test --------- Co-authored-by: ofekshenawa <[email protected]> * SortByWithCount FTSearchOptions fix (#3201) * SortByWithCount FTSearchOptions fix * FTSearch test fix * Another FTSearch test fix * Another FTSearch test fix --------- Co-authored-by: Christopher Golling <[email protected]> * Fix race condition in clusterNodes.Addrs() (#3219) Resolve a race condition in the clusterNodes.Addrs() method. Previously, the method returned a reference to a string slice, creating the potential for concurrent reads by the caller while the slice was being modified by the garbage collection process. Co-authored-by: Nedyalko Dyakov <[email protected]> * chore: fix some comments (#3226) Signed-off-by: zhuhaicity <[email protected]> Co-authored-by: Nedyalko Dyakov <[email protected]> * fix(aggregate, search): ft.aggregate bugfixes (#3263) * fix: rearange args for ft.aggregate apply should be before any groupby or sortby * improve test * wip: add scorer and addscores * enable all tests * fix ftsearch with count test * make linter happy * Addscores is available in later redisearch releases. For safety state it is available in redis ce 8 * load an apply seem to break scorer and addscores * fix: add unstableresp3 to cluster client (#3266) * fix: add unstableresp3 to cluster client * propagate unstableresp3 * proper test that will ignore error, but fail if client panics * add separate test for clusterclient constructor * fix: flaky ClientKillByFilter test (#3268) * Reinstate read-only lock on hooks access in dialHook (#3225) * use limit when limitoffset is zero (#3275) * remove redis 8 comments * update package versions * use latest golangci-lint * fix(search&aggregate):fix error overwrite and typo #3220 (#3224) * fix (#3220) * LOAD has NO AS param(https://redis.io/docs/latest/commands/ft.aggregate/) * fix typo: WITHCOUT -> WITHCOUNT * fix (#3220): * Compatible with known RediSearch issue in test * fix (#3220) * fixed the calculation bug of the count of load params * test should not include special condition * return errors when they occur --------- Co-authored-by: Nedyalko Dyakov <[email protected]> Co-authored-by: ofekshenawa <[email protected]> * Recognize byte slice for key argument in cluster client hash slot computation (#3049) Co-authored-by: Vladyslav Vildanov <[email protected]> Co-authored-by: ofekshenawa <[email protected]> --------- Signed-off-by: zhuhaicity <[email protected]> Co-authored-by: ofekshenawa <[email protected]> Co-authored-by: LINKIWI <[email protected]> Co-authored-by: Cgol9 <[email protected]> Co-authored-by: Christopher Golling <[email protected]> Co-authored-by: Shawn Wang <[email protected]> Co-authored-by: ZhuHaiCheng <[email protected]> Co-authored-by: herodot <[email protected]> Co-authored-by: Vladyslav Vildanov <[email protected]>
redis · Feb 21, 2025 · 3d041a1 · 3d041a1
1 parent ed37c33
commit 3d041a1
Show file tree

Hide file tree

Showing 27 changed files with 365 additions and 72 deletions.
diff --git a/.github/wordlist.txt b/.github/wordlist.txt
@@ -54,6 +54,7 @@ stunnel
 SynDump
 TCP
 TLS
+UnstableResp
 uri
 URI
 url
@@ -62,3 +63,5 @@ RedisStack
 RedisGears
 RedisTimeseries
 RediSearch
+RawResult
+RawVal
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -12,15 +12,13 @@ on:
 
 permissions:
   contents: read
+  pull-requests: read  # for golangci/golangci-lint-action to fetch pull requests
 
 jobs:
   golangci:
-    permissions:
-      contents: read  # for actions/checkout to fetch code
-      pull-requests: read  # for golangci/golangci-lint-action to fetch pull requests
     name: lint
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v6
+        uses: golangci/golangci-lint-action@v6.5.0
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,4 +1,3 @@
 run:
-  concurrency: 8
-  deadline: 5m
+  timeout: 5m
   tests: false
diff --git a/README.md b/README.md
@@ -186,6 +186,21 @@ rdb := redis.NewClient(&redis.Options{
 #### Unstable RESP3 Structures for RediSearch Commands
 When integrating Redis with application functionalities using RESP3, it's important to note that some response structures aren't final yet. This is especially true for more complex structures like search and query results. We recommend using RESP2 when using the search and query capabilities, but we plan to stabilize the RESP3-based API-s in the coming versions. You can find more guidance in the upcoming release notes.
 
+To enable unstable RESP3, set the option in your client configuration:
+
+```go
+redis.NewClient(&redis.Options{
+			UnstableResp3: true,
+		})
+```
+**Note:** When UnstableResp3 mode is enabled, it's necessary to use RawResult() and RawVal() to retrieve a raw data.
+          Since, raw response is the only option for unstable search commands Val() and Result() calls wouldn't have any affect on them:
+
+```go
+res1, err := client.FTSearchWithArgs(ctx, "txt", "foo bar", &redis.FTSearchOptions{}).RawResult()
+val1 := client.FTSearchWithArgs(ctx, "txt", "foo bar", &redis.FTSearchOptions{}).RawVal()
+```
+
 ## Contributing
 
 Please see [out contributing guidelines](CONTRIBUTING.md) to help us improve this library!

diff --git a/command.go b/command.go
@@ -167,6 +167,8 @@ func (cmd *baseCmd) stringArg(pos int) string {
 	switch v := arg.(type) {
 	case string:
 		return v
+	case []byte:
+		return string(v)
 	default:
 		// TODO: consider using appendArg
 		return fmt.Sprint(v)

diff --git a/commands_test.go b/commands_test.go
@@ -217,7 +217,7 @@ var _ = Describe("Commands", func() {
 
 			killed := client.ClientKillByFilter(ctx, "MAXAGE", "1")
 			Expect(killed.Err()).NotTo(HaveOccurred())
-			Expect(killed.Val()).To(SatisfyAny(Equal(int64(2)), Equal(int64(3))))
+			Expect(killed.Val()).To(BeNumerically(">=", 2))
 
 			select {
 			case <-done:

diff --git a/example/del-keys-without-ttl/go.mod b/example/del-keys-without-ttl/go.mod
@@ -5,7 +5,7 @@ go 1.18
 replace github.com/redis/go-redis/v9 => ../..
 
 require (
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/v9 v9.7.1
 	go.uber.org/zap v1.24.0
 )
 

diff --git a/example/hll/go.mod b/example/hll/go.mod
@@ -4,7 +4,7 @@ go 1.18
 
 replace github.com/redis/go-redis/v9 => ../..
 
-require github.com/redis/go-redis/v9 v9.7.0
+require github.com/redis/go-redis/v9 v9.7.1
 
 require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect

diff --git a/example/lua-scripting/go.mod b/example/lua-scripting/go.mod
@@ -4,7 +4,7 @@ go 1.18
 
 replace github.com/redis/go-redis/v9 => ../..
 
-require github.com/redis/go-redis/v9 v9.7.0
+require github.com/redis/go-redis/v9 v9.7.1
 
 require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect

diff --git a/example/otel/go.mod b/example/otel/go.mod
@@ -9,8 +9,8 @@ replace github.com/redis/go-redis/extra/redisotel/v9 => ../../extra/redisotel
 replace github.com/redis/go-redis/extra/rediscmd/v9 => ../../extra/rediscmd
 
 require (
-	github.com/redis/go-redis/extra/redisotel/v9 v9.7.0
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/extra/redisotel/v9 v9.7.1
+	github.com/redis/go-redis/v9 v9.7.1
 	github.com/uptrace/uptrace-go v1.21.0
 	go.opentelemetry.io/otel v1.22.0
 )
@@ -23,7 +23,7 @@ require (
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.0 // indirect
-	github.com/redis/go-redis/extra/rediscmd/v9 v9.7.0 // indirect
+	github.com/redis/go-redis/extra/rediscmd/v9 v9.7.1 // indirect
 	go.opentelemetry.io/contrib/instrumentation/runtime v0.46.1 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.44.0 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.21.0 // indirect

diff --git a/example/redis-bloom/go.mod b/example/redis-bloom/go.mod
@@ -4,7 +4,7 @@ go 1.18
 
 replace github.com/redis/go-redis/v9 => ../..
 
-require github.com/redis/go-redis/v9 v9.7.0
+require github.com/redis/go-redis/v9 v9.7.1
 
 require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect

diff --git a/example/scan-struct/go.mod b/example/scan-struct/go.mod
@@ -6,7 +6,7 @@ replace github.com/redis/go-redis/v9 => ../..
 
 require (
 	github.com/davecgh/go-spew v1.1.1
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/v9 v9.7.1
 )
 
 require (

diff --git a/extra/rediscensus/go.mod b/extra/rediscensus/go.mod
@@ -7,8 +7,8 @@ replace github.com/redis/go-redis/v9 => ../..
 replace github.com/redis/go-redis/extra/rediscmd/v9 => ../rediscmd
 
 require (
-	github.com/redis/go-redis/extra/rediscmd/v9 v9.7.0
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/extra/rediscmd/v9 v9.7.1
+	github.com/redis/go-redis/v9 v9.7.1
 	go.opencensus.io v0.24.0
 )
 

diff --git a/extra/rediscmd/go.mod b/extra/rediscmd/go.mod
@@ -7,7 +7,7 @@ replace github.com/redis/go-redis/v9 => ../..
 require (
 	github.com/bsm/ginkgo/v2 v2.12.0
 	github.com/bsm/gomega v1.27.10
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/v9 v9.7.1
 )
 
 require (

diff --git a/extra/redisotel/go.mod b/extra/redisotel/go.mod
@@ -7,8 +7,8 @@ replace github.com/redis/go-redis/v9 => ../..
 replace github.com/redis/go-redis/extra/rediscmd/v9 => ../rediscmd
 
 require (
-	github.com/redis/go-redis/extra/rediscmd/v9 v9.7.0
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/extra/rediscmd/v9 v9.7.1
+	github.com/redis/go-redis/v9 v9.7.1
 	go.opentelemetry.io/otel v1.22.0
 	go.opentelemetry.io/otel/metric v1.22.0
 	go.opentelemetry.io/otel/sdk v1.22.0

diff --git a/extra/redisprometheus/go.mod b/extra/redisprometheus/go.mod
@@ -6,7 +6,7 @@ replace github.com/redis/go-redis/v9 => ../..
 
 require (
 	github.com/prometheus/client_golang v1.14.0
-	github.com/redis/go-redis/v9 v9.7.0
+	github.com/redis/go-redis/v9 v9.7.1
 )
 
 require (

diff --git a/hash_commands.go b/hash_commands.go
@@ -225,7 +225,7 @@ func (c cmdable) HExpire(ctx context.Context, key string, expiration time.Durati
 	return cmd
 }
 
-// HExpire - Sets the expiration time for specified fields in a hash in seconds.
+// HExpireWithArgs - Sets the expiration time for specified fields in a hash in seconds.
 // It requires a key, an expiration duration, a struct with boolean flags for conditional expiration settings (NX, XX, GT, LT), and a list of fields.
 // The command constructs an argument list starting with "HEXPIRE", followed by the key, duration, any conditional flags, and the specified fields.
 // For more information - https://redis.io/commands/hexpire/

diff --git a/options.go b/options.go
@@ -154,7 +154,7 @@ type Options struct {
 	// Add suffix to client name. Default is empty.
 	IdentitySuffix string
 
-	// Enable Unstable mode for Redis Search module with RESP3.
+	// UnstableResp3 enables Unstable mode for Redis Search module with RESP3.
 	UnstableResp3 bool
 }
 

diff --git a/osscluster.go b/osscluster.go
@@ -90,6 +90,9 @@ type ClusterOptions struct {
 	DisableIndentity bool // Disable set-lib on connect. Default is false.
 
 	IdentitySuffix string // Add suffix to client name. Default is empty.
+
+	// UnstableResp3 enables Unstable mode for Redis Search module with RESP3.
+	UnstableResp3 bool
 }
 
 func (opt *ClusterOptions) init() {
@@ -304,7 +307,8 @@ func (opt *ClusterOptions) clientOptions() *Options {
 		// much use for ClusterSlots config).  This means we cannot execute the
 		// READONLY command against that node -- setting readOnly to false in such
 		// situations in the options below will prevent that from happening.
-		readOnly: opt.ReadOnly && opt.ClusterSlots == nil,
+		readOnly:      opt.ReadOnly && opt.ClusterSlots == nil,
+		UnstableResp3: opt.UnstableResp3,
 	}
 }
 
@@ -465,9 +469,11 @@ func (c *clusterNodes) Addrs() ([]string, error) {
 	closed := c.closed //nolint:ifshort
 	if !closed {
 		if len(c.activeAddrs) > 0 {
-			addrs = c.activeAddrs
+			addrs = make([]string, len(c.activeAddrs))
+			copy(addrs, c.activeAddrs)
 		} else {
-			addrs = c.addrs
+			addrs = make([]string, len(c.addrs))
+			copy(addrs, c.addrs)
 		}
 	}
 	c.mu.RUnlock()

diff --git a/osscluster_test.go b/osscluster_test.go
@@ -653,6 +653,32 @@ var _ = Describe("ClusterClient", func() {
 			Expect(client.Close()).NotTo(HaveOccurred())
 		})
 
+		It("determines hash slots correctly for generic commands", func() {
+			opt := redisClusterOptions()
+			opt.MaxRedirects = -1
+			client := cluster.newClusterClient(ctx, opt)
+
+			err := client.Do(ctx, "GET", "A").Err()
+			Expect(err).To(Equal(redis.Nil))
+
+			err = client.Do(ctx, []byte("GET"), []byte("A")).Err()
+			Expect(err).To(Equal(redis.Nil))
+
+			Eventually(func() error {
+				return client.SwapNodes(ctx, "A")
+			}, 30*time.Second).ShouldNot(HaveOccurred())
+
+			err = client.Do(ctx, "GET", "A").Err()
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("MOVED"))
+
+			err = client.Do(ctx, []byte("GET"), []byte("A")).Err()
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("MOVED"))
+
+			Expect(client.Close()).NotTo(HaveOccurred())
+		})
+
 		It("follows node redirection immediately", func() {
 			// Configure retry backoffs far in excess of the expected duration of redirection
 			opt := redisClusterOptions()

diff --git a/redis.go b/redis.go
@@ -41,15 +41,15 @@ type (
 )
 
 type hooksMixin struct {
-	hooksMu *sync.Mutex
+	hooksMu *sync.RWMutex
 
 	slice   []Hook
 	initial hooks
 	current hooks
 }
 
 func (hs *hooksMixin) initHooks(hooks hooks) {
-	hs.hooksMu = new(sync.Mutex)
+	hs.hooksMu = new(sync.RWMutex)
 	hs.initial = hooks
 	hs.chain()
 }
@@ -151,7 +151,7 @@ func (hs *hooksMixin) clone() hooksMixin {
 	clone := *hs
 	l := len(clone.slice)
 	clone.slice = clone.slice[:l:l]
-	clone.hooksMu = new(sync.Mutex)
+	clone.hooksMu = new(sync.RWMutex)
 	return clone
 }
 
@@ -176,9 +176,14 @@ func (hs *hooksMixin) withProcessPipelineHook(
 }
 
 func (hs *hooksMixin) dialHook(ctx context.Context, network, addr string) (net.Conn, error) {
-	hs.hooksMu.Lock()
-	defer hs.hooksMu.Unlock()
-	return hs.current.dial(ctx, network, addr)
+	// Access to hs.current is guarded by a read-only lock since it may be mutated by AddHook(...)
+	// while this dialer is concurrently accessed by the background connection pool population
+	// routine when MinIdleConns > 0.
+	hs.hooksMu.RLock()
+	current := hs.current
+	hs.hooksMu.RUnlock()
+
+	return current.dial(ctx, network, addr)
 }
 
 func (hs *hooksMixin) processHook(ctx context.Context, cmd Cmder) error {

diff --git a/redis_test.go b/redis_test.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"sync"
 	"testing"
 	"time"
 
@@ -633,3 +634,67 @@ var _ = Describe("Hook with MinIdleConns", func() {
 		}))
 	})
 })
+
+var _ = Describe("Dialer connection timeouts", func() {
+	var client *redis.Client
+
+	const dialSimulatedDelay = 1 * time.Second
+
+	BeforeEach(func() {
+		options := redisOptions()
+		options.Dialer = func(ctx context.Context, network, addr string) (net.Conn, error) {
+			// Simulated slow dialer.
+			// Note that the following sleep is deliberately not context-aware.
+			time.Sleep(dialSimulatedDelay)
+			return net.Dial("tcp", options.Addr)
+		}
+		options.MinIdleConns = 1
+		client = redis.NewClient(options)
+	})
+
+	AfterEach(func() {
+		err := client.Close()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("does not contend on connection dial for concurrent commands", func() {
+		var wg sync.WaitGroup
+
+		const concurrency = 10
+
+		durations := make(chan time.Duration, concurrency)
+		errs := make(chan error, concurrency)
+
+		start := time.Now()
+		wg.Add(concurrency)
+
+		for i := 0; i < concurrency; i++ {
+			go func() {
+				defer wg.Done()
+
+				start := time.Now()
+				err := client.Ping(ctx).Err()
+				durations <- time.Since(start)
+				errs <- err
+			}()
+		}
+
+		wg.Wait()
+		close(durations)
+		close(errs)
+
+		// All commands should eventually succeed, after acquiring a connection.
+		for err := range errs {
+			Expect(err).NotTo(HaveOccurred())
+		}
+
+		// Each individual command should complete within the simulated dial duration bound.
+		for duration := range durations {
+			Expect(duration).To(BeNumerically("<", 2*dialSimulatedDelay))
+		}
+
+		// Due to concurrent execution, the entire test suite should also complete within
+		// the same dial duration bound applied for individual commands.
+		Expect(time.Since(start)).To(BeNumerically("<", 2*dialSimulatedDelay))
+	})
+})