From 5756b05219006501a3c815fcb378b1eaa303c203 Mon Sep 17 00:00:00 2001 From: LINKIWI 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 73a9e2b7..ce258ff3 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 3d2f8071..f7bd1683 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())