Skip to content

Commit

Permalink
util: fix retry.WithMaxAttempts context cancelled before run.
Browse files Browse the repository at this point in the history
If context gets cancelled right after `retry.WithMaxAttempts` runs, the function
passed to it will never gets run. Now `retry.WithMaxAttempts` will at least run
the function once otherwise an error will be returned.

Making this change because there are places such as `show_cluster_setting.go`
require the passed in function to be run at least once. Otherwise there will
be seg fault.

Fixes: 25600, 25603, 25570, 25567, 25566, 25511, 25485.
Release note: None
  • Loading branch information
windchan7 committed May 17, 2018
1 parent fcc4eb4 commit 129113c
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions pkg/util/retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/pkg/errors"
)

// Options provides reusable configuration of Retry objects.
Expand Down Expand Up @@ -150,7 +151,12 @@ func (r *Retry) NextCh() <-chan time.Time {
}

// WithMaxAttempts is a helper that runs fn N times and collects the last err.
// It guarantees fn will run at least once. Otherwise, an error will be returned.
func WithMaxAttempts(ctx context.Context, opts Options, n int, fn func() error) error {
if n <= 0 {
return errors.Errorf("max attempts should not be 0 or below, got: %d", n)
}

opts.MaxRetries = n - 1
var err error
for r := StartWithCtx(ctx, opts); r.Next(); {
Expand All @@ -159,6 +165,9 @@ func WithMaxAttempts(ctx context.Context, opts Options, n int, fn func() error)
return nil
}
}
if err == nil {
err = errors.Wrap(ctx.Err(), "did not run function")
}
return err
}

Expand Down

0 comments on commit 129113c

Please sign in to comment.