Skip to content

Commit

Permalink
Avoid unnecessary retry delay following MOVED and ASK redirection (#3048
Browse files Browse the repository at this point in the history
)
  • Loading branch information
LINKIWI authored and vladvildanov committed Jul 17, 2024
1 parent 233ff45 commit 2c146dc
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
6 changes: 4 additions & 2 deletions osscluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 26 additions & 0 deletions osscluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit 2c146dc

Please sign in to comment.