From 64f8f47a37d1711af82bd380940bf02eae26b341 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Tue, 21 Dec 2021 15:04:23 -0500 Subject: [PATCH] Prevent Linear Retry from converging on Max (#9449) Backport of #9393 --- lib/utils/retry.go | 3 ++ lib/utils/retry_test.go | 91 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 lib/utils/retry_test.go diff --git a/lib/utils/retry.go b/lib/utils/retry.go index 4a52ed5ab92a5..cc58db97c8c3b 100644 --- a/lib/utils/retry.go +++ b/lib/utils/retry.go @@ -220,6 +220,9 @@ func (r *Linear) Duration() time.Duration { if a <= r.Max { return a } + if r.Jitter != nil { + return r.Jitter(r.Max) + } return r.Max } diff --git a/lib/utils/retry_test.go b/lib/utils/retry_test.go new file mode 100644 index 0000000000000..87ae31a389d7e --- /dev/null +++ b/lib/utils/retry_test.go @@ -0,0 +1,91 @@ +/* +Copyright 2021 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func Test_LinearRetryMax(t *testing.T) { + t.Parallel() + cases := []struct { + desc string + config LinearConfig + previousCompareFn require.ComparisonAssertionFunc + }{ + { + desc: "HalfJitter", + config: LinearConfig{ + First: time.Second * 45, + Step: time.Second * 30, + Max: time.Minute, + Jitter: NewJitter(), + }, + previousCompareFn: require.NotEqual, + }, + { + desc: "SeventhJitter", + config: LinearConfig{ + First: time.Second * 45, + Step: time.Second * 30, + Max: time.Minute, + Jitter: NewSeventhJitter(), + }, + previousCompareFn: require.NotEqual, + }, + { + desc: "NoJitter", + config: LinearConfig{ + First: time.Second * 45, + Step: time.Second * 30, + Max: time.Minute, + }, + previousCompareFn: require.Equal, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + linear, err := NewLinear(tc.config) + require.NoError(t, err) + + // artificially spike the attempts to get to max + linear.attempt = 100 + + // get the initial previous value to compare with + previous := linear.Duration() + linear.Inc() + + for i := 0; i < 50; i++ { + duration := linear.Duration() + linear.Inc() + + // ensure duration does not exceed maximum + require.LessOrEqual(t, duration, tc.config.Max) + + // ensure duration comparison to previous is satisfied + tc.previousCompareFn(t, duration, previous) + + } + }) + } +}