Skip to content

Commit

Permalink
vmctl: finish retries if context canceled (#4442)
Browse files Browse the repository at this point in the history
vmctl: interrupt backoff retries if import context is cancelled

Co-authored-by: Roman Khavronenko <roman@victoriametrics.com>
  • Loading branch information
2 people authored and valyala committed Jul 6, 2023
1 parent 0e0b7bf commit 2e81c5f
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 3 deletions.
15 changes: 13 additions & 2 deletions app/vmctl/backoff/backoff.go
Expand Up @@ -42,7 +42,6 @@ func New() *Backoff {
func (b *Backoff) Retry(ctx context.Context, cb retryableFunc) (uint64, error) {
var attempt uint64
for i := 0; i < b.retries; i++ {
// @TODO we should use context to cancel retries
err := cb()
if err == nil {
return attempt, nil
Expand All @@ -55,7 +54,19 @@ func (b *Backoff) Retry(ctx context.Context, cb retryableFunc) (uint64, error) {
backoff := float64(b.minDuration) * math.Pow(b.factor, float64(i))
dur := time.Duration(backoff)
logger.Errorf("got error: %s on attempt: %d; will retry in %v", err, attempt, dur)
time.Sleep(time.Duration(backoff))

t := time.NewTimer(dur)
select {
case <-t.C:
// duration elapsed, loop
case <-ctx.Done():
// context cancelled, kill the timer if it hasn't fired, and return
// the last error we got
if !t.Stop() {
<-t.C
}
return attempt, err
}
}
return attempt, fmt.Errorf("execution failed after %d retry attempts", b.retries)
}
25 changes: 24 additions & 1 deletion app/vmctl/backoff/backoff_test.go
Expand Up @@ -16,7 +16,7 @@ func TestRetry_Do(t *testing.T) {
backoffMinDuration time.Duration
retryableFunc retryableFunc
ctx context.Context
withCancel bool
cancelTimeout time.Duration
want uint64
wantErr bool
}{
Expand Down Expand Up @@ -79,10 +79,33 @@ func TestRetry_Do(t *testing.T) {
want: 5,
wantErr: true,
},
{
name: "cancel context",
backoffRetries: 5,
backoffFactor: 0.1,
backoffMinDuration: time.Millisecond * 10,
retryableFunc: func() error {
t := time.NewTicker(time.Millisecond * 5)
defer t.Stop()
for range t.C {
return fmt.Errorf("got some error")
}
return nil
},
ctx: context.Background(),
cancelTimeout: time.Second * 5,
want: 3,
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := New()
if tt.cancelTimeout != 0 {
newCtx, cancelFn := context.WithTimeout(tt.ctx, tt.cancelTimeout)
tt.ctx = newCtx
defer cancelFn()
}
got, err := r.Retry(tt.ctx, tt.retryableFunc)
if (err != nil) != tt.wantErr {
t.Errorf("Retry() error = %v, wantErr %v", err, tt.wantErr)
Expand Down
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Expand Up @@ -27,6 +27,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
* SECURITY: upgrade Go builder from Go1.20.4 to Go1.20.5. See [the list of issues addressed in Go1.20.5](https://github.com/golang/go/issues?q=milestone%3AGo1.20.5+label%3ACherryPickApproved).

* FEATURE: [vmctl](https://docs.victoriametrics.com/vmctl.html): add verbose output for docker installations or when TTY isn't available. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4081).
* FEATURE: [vmctl](https://docs.victoriametrics.com/vmctl.html): interrupt backoff retries when import process is cancelled. The change makes vmctl more responsive in case of errors during the import. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4442).
* FEATURE: vmstorage: suppress "broken pipe" errors for search queries on vmstorage side. See [this commit](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4418/commits/a6a7795b9e1f210d614a2c5f9a3016b97ded4792).
* FEATURE: [Official Grafana dashboards for VictoriaMetrics](https://grafana.com/orgs/victoriametrics): add panel for tracking rate of syscalls while writing or reading from disk via `process_io_(read|write)_syscalls_total` metrics.
* FEATURE: add ability to fine-tune Graphite API limits via the following command-line flags:
Expand Down

0 comments on commit 2e81c5f

Please sign in to comment.