Merge pull request #2025 from benluddy/small-jitters

Bug 1932182: Support jittering relatively small resync intervals.

Author: openshift-merge-robot

pkg/lib/queueinformer/jitter.go

@@ -1,32 +1,45 @@
 package queueinformer
 
 import (
+	"math"
 	"math/rand"
 	"time"
 )
 
 const DefaultResyncPeriod = 15 * time.Minute
 
+type float64er interface {
+	// Float64 returns a float64 in range [0.0, 1.0).
+	Float64() float64
+}
+
+type realFloat64er struct{}
+
+func (realFloat64er) Float64() float64 {
+	return rand.Float64()
+}
+
 // ResyncWithJitter takes a resync interval and adds jitter within a percent difference.
 // factor is a value between 0 and 1 indicating the amount of jitter
 // a factor of 0.2 and a period of 10m will have a range of 8 to 12 minutes (20%)
 func ResyncWithJitter(resyncPeriod time.Duration, factor float64) func() time.Duration {
+	return resyncWithJitter(resyncPeriod, factor, realFloat64er{})
+}
+
+func resyncWithJitter(period time.Duration, factor float64, rand float64er) func() time.Duration {
 	return func() time.Duration {
-		if factor < 0.0 || factor > 1.0 {
-			return resyncPeriod
-		}
-		if resyncPeriod < 0.0 {
+		if period < 0.0 {
 			return DefaultResyncPeriod
 		}
-
-		// if we would wrap around, return resyncPeriod
-		if time.Duration((1+factor)*resyncPeriod.Minutes())*time.Minute < 0.0 {
-			return resyncPeriod
+		if period > math.MaxInt64/2 { // 1281023h53m38.427387903s
+			// avoid overflowing time.Duration
+			return period
+		}
+		if factor < 0.0 || factor > 1.0 {
+			return period
 		}
 
-		min := resyncPeriod.Minutes() * (1 - factor)
-		max := resyncPeriod.Minutes() * (1 + factor)
-
-		return time.Duration(min)*time.Minute + time.Duration(rand.Float64()*(max-min))*time.Minute
+		// The effective scale will be in [1-factor, 1+factor) because rand.Float64() is in [0.0, 1.0).
+		return time.Duration((1 - factor + 2*rand.Float64()*factor) * float64(period))
 	}
 }

pkg/lib/queueinformer/jitter_test.go

@@ -1,6 +1,7 @@
 package queueinformer
 
 import (
+	"math"
 	"math/rand"
 	"reflect"
 	"testing"
@@ -10,60 +11,134 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+type fakeFloat64er float64
+
+func (f fakeFloat64er) Float64() float64 {
+	return float64(f)
+}
+
 func TestResyncWithJitter(t *testing.T) {
 	type args struct {
 		resyncPeriod time.Duration
 		factor       float64
+		r            float64
 	}
 	tests := []struct {
-		name    string
-		args    args
-		wantMin time.Duration
-		wantMax time.Duration
+		name string
+		args args
+		want time.Duration
 	}{
 		{
-			name: "TypicalInput/Minutes",
+			name: "TypicalInput/15Minutes/Min",
+			args: args{
+				resyncPeriod: 15 * time.Minute,
+				factor:       0.2,
+				r:            0,
+			},
+			want: 12 * time.Minute,
+		},
+		{
+			name: "TypicalInput/15Minutes/Mid",
+			args: args{
+				resyncPeriod: 15 * time.Minute,
+				factor:       0.2,
+				r:            0.5,
+			},
+			want: 15 * time.Minute,
+		},
+		{
+			name: "TypicalInput/15Minutes/Max",
 			args: args{
 				resyncPeriod: 15 * time.Minute,
 				factor:       0.2,
+				r:            1,
 			},
-			wantMin: 12 * time.Minute,
-			wantMax: 18 * time.Minute,
+			want: 18 * time.Minute,
 		},
 		{
-			name: "TypicalInput/Hours",
+			name: "TypicalInput/10Hours/Min",
 			args: args{
 				resyncPeriod: 10 * time.Hour,
 				factor:       0.1,
+				r:            0,
 			},
-			wantMin: 9 * time.Hour,
-			wantMax: 11 * time.Hour,
+			want: 9 * time.Hour,
+		},
+		{
+			name: "TypicalInput/10Hours/Mid",
+			args: args{
+				resyncPeriod: 10 * time.Hour,
+				factor:       0.1,
+				r:            0.5,
+			},
+			want: 10 * time.Hour,
+		},
+		{
+			name: "TypicalInput/10Hours/Max",
+			args: args{
+				resyncPeriod: 10 * time.Hour,
+				factor:       0.1,
+				r:            1,
+			},
+			want: 11 * time.Hour,
 		},
 		{
 			name: "BadInput/BadFactor",
 			args: args{
 				resyncPeriod: 10 * time.Hour,
 				factor:       -0.1,
 			},
-			wantMin: 10 * time.Hour,
-			wantMax: 10 * time.Hour,
+			want: 10 * time.Hour,
 		},
 		{
 			name: "BadInput/BadResync",
 			args: args{
 				resyncPeriod: -10 * time.Hour,
 				factor:       0.1,
 			},
-			wantMin: DefaultResyncPeriod,
-			wantMax: DefaultResyncPeriod,
+			want: DefaultResyncPeriod,
+		},
+		{
+			name: "BadInput/Big",
+			args: args{
+				resyncPeriod: time.Duration(math.MaxInt64),
+				factor:       1,
+				r:            1,
+			},
+			want: time.Duration(math.MaxInt64),
+		},
+		{
+			name: "SmallInput/Min",
+			args: args{
+				resyncPeriod: 10 * time.Second,
+				factor:       0.5,
+				r:            0,
+			},
+			want: 5 * time.Second,
+		},
+		{
+			name: "SmallInput/Mid",
+			args: args{
+				resyncPeriod: 10 * time.Second,
+				factor:       0.5,
+				r:            0.5,
+			},
+			want: 10 * time.Second,
+		},
+		{
+			name: "SmallInput/Max",
+			args: args{
+				resyncPeriod: 10 * time.Second,
+				factor:       0.5,
+				r:            1,
+			},
+			want: 15 * time.Second,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := ResyncWithJitter(tt.args.resyncPeriod, tt.args.factor)
-			require.True(t, got() >= tt.wantMin)
-			require.True(t, got() <= tt.wantMax)
-			require.True(t, got() != got() || tt.wantMax == tt.wantMin)
+			got := resyncWithJitter(tt.args.resyncPeriod, tt.args.factor, fakeFloat64er(tt.args.r))()
+			require.Equal(t, tt.want, got)
 		})
 	}
 }