Skip to content

Commit 5756b05

Browse files
authored
Avoid unnecessary retry delay following MOVED and ASK redirection (#3048)
1 parent 0858ed2 commit 5756b05

File tree

2 files changed

+30
-2
lines changed

2 files changed

+30
-2
lines changed

Diff for: osscluster.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -938,10 +938,13 @@ func (c *ClusterClient) Process(ctx context.Context, cmd Cmder) error {
938938
func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error {
939939
slot := c.cmdSlot(ctx, cmd)
940940
var node *clusterNode
941+
var moved bool
941942
var ask bool
942943
var lastErr error
943944
for attempt := 0; attempt <= c.opt.MaxRedirects; attempt++ {
944-
if attempt > 0 {
945+
// MOVED and ASK responses are not transient errors that require retry delay; they
946+
// should be attempted immediately.
947+
if attempt > 0 && !moved && !ask {
945948
if err := internal.Sleep(ctx, c.retryBackoff(attempt)); err != nil {
946949
return err
947950
}
@@ -985,7 +988,6 @@ func (c *ClusterClient) process(ctx context.Context, cmd Cmder) error {
985988
continue
986989
}
987990

988-
var moved bool
989991
var addr string
990992
moved, ask, addr = isMovedError(lastErr)
991993
if moved || ask {

Diff for: osscluster_test.go

+26
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,32 @@ var _ = Describe("ClusterClient", func() {
653653
Expect(client.Close()).NotTo(HaveOccurred())
654654
})
655655

656+
It("follows node redirection immediately", func() {
657+
// Configure retry backoffs far in excess of the expected duration of redirection
658+
opt := redisClusterOptions()
659+
opt.MinRetryBackoff = 10 * time.Minute
660+
opt.MaxRetryBackoff = 20 * time.Minute
661+
client := cluster.newClusterClient(ctx, opt)
662+
663+
Eventually(func() error {
664+
return client.SwapNodes(ctx, "A")
665+
}, 30*time.Second).ShouldNot(HaveOccurred())
666+
667+
// Note that this context sets a deadline more aggressive than the lowest possible bound
668+
// of the retry backoff; this verifies that redirection completes immediately.
669+
redirCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
670+
defer cancel()
671+
672+
err := client.Set(redirCtx, "A", "VALUE", 0).Err()
673+
Expect(err).NotTo(HaveOccurred())
674+
675+
v, err := client.Get(redirCtx, "A").Result()
676+
Expect(err).NotTo(HaveOccurred())
677+
Expect(v).To(Equal("VALUE"))
678+
679+
Expect(client.Close()).NotTo(HaveOccurred())
680+
})
681+
656682
It("calls fn for every master node", func() {
657683
for i := 0; i < 10; i++ {
658684
Expect(client.Set(ctx, strconv.Itoa(i), "", 0).Err()).NotTo(HaveOccurred())

0 commit comments

Comments
 (0)