From 2c146dc1042d078bb6ef2ea76f35d7d4a0212357 Mon Sep 17 00:00:00 2001
From: LINKIWI <LINKIWI@users.noreply.github.com>
Date: Fri, 12 Jul 2024 23:55:12 -0700
Subject: [PATCH] Avoid unnecessary retry delay following MOVED and ASK
 redirection (#3048)

---
 osscluster.go      |  6 ++++--
 osscluster_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/osscluster.go b/osscluster.go
index 73a9e2b74..ce258ff36 100644
--- a/osscluster.go
+++ b/osscluster.go
@@ -938,10 +938,13 @@ func (c *ClusterClient) Process(ctx context.Context, cmd Cmder) error {
 func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error {
 	slot := c.cmdSlot(ctx, cmd)
 	var node *clusterNode
+	var moved bool
 	var ask bool
 	var lastErr error
 	for attempt := 0; attempt <= c.opt.MaxRedirects; attempt++ {
-		if attempt > 0 {
+		// MOVED and ASK responses are not transient errors that require retry delay; they
+		// should be attempted immediately.
+		if attempt > 0 && !moved && !ask {
 			if err := internal.Sleep(ctx, c.retryBackoff(attempt)); err != nil {
 				return err
 			}
@@ -985,7 +988,6 @@ func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error {
 			continue
 		}
 
-		var moved bool
 		var addr string
 		moved, ask, addr = isMovedError(lastErr)
 		if moved || ask {
diff --git a/osscluster_test.go b/osscluster_test.go
index 3d2f80711..f7bd1683f 100644
--- a/osscluster_test.go
+++ b/osscluster_test.go
@@ -653,6 +653,32 @@ var _ = Describe("ClusterClient", func() {
 			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()
+			opt.MinRetryBackoff = 10 * time.Minute
+			opt.MaxRetryBackoff = 20 * time.Minute
+			client := cluster.newClusterClient(ctx, opt)
+
+			Eventually(func() error {
+				return client.SwapNodes(ctx, "A")
+			}, 30*time.Second).ShouldNot(HaveOccurred())
+
+			// Note that this context sets a deadline more aggressive than the lowest possible bound
+			// of the retry backoff; this verifies that redirection completes immediately.
+			redirCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+			defer cancel()
+
+			err := client.Set(redirCtx, "A", "VALUE", 0).Err()
+			Expect(err).NotTo(HaveOccurred())
+
+			v, err := client.Get(redirCtx, "A").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(v).To(Equal("VALUE"))
+
+			Expect(client.Close()).NotTo(HaveOccurred())
+		})
+
 		It("calls fn for every master node", func() {
 			for i := 0; i < 10; i++ {
 				Expect(client.Set(ctx, strconv.Itoa(i), "", 0).Err()).NotTo(HaveOccurred())