From bae10246dd236c0f88dbccfa0ba6785e4f01e8d6 Mon Sep 17 00:00:00 2001
From: Marcel Zieba <marcel.zieba@isovalent.com>
Date: Fri, 19 May 2023 14:28:31 +0200
Subject: [PATCH] Improve backoff policy in reflector.

Before, we've used two separate backoff managers for List and Watch
calls, now they share single backoff manager.

Kubernetes-commit: 337728b02559dec8a613fdef174f732da9cae310
---
 tools/cache/reflector.go      | 19 ++++++++----------
 tools/cache/reflector_test.go | 36 +++++++++++++++++------------------
 2 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/tools/cache/reflector.go b/tools/cache/reflector.go
index 400d4015..8d015833 100644
--- a/tools/cache/reflector.go
+++ b/tools/cache/reflector.go
@@ -70,9 +70,7 @@ type Reflector struct {
 	listerWatcher ListerWatcher
 	// backoff manages backoff of ListWatch
 	backoffManager wait.BackoffManager
-	// initConnBackoffManager manages backoff the initial connection with the Watch call of ListAndWatch.
-	initConnBackoffManager wait.BackoffManager
-	resyncPeriod           time.Duration
+	resyncPeriod   time.Duration
 	// clock allows tests to manipulate time
 	clock clock.Clock
 	// paginatedResult defines whether pagination should be forced for list calls.
@@ -221,11 +219,10 @@ func NewReflectorWithOptions(lw ListerWatcher, expectedType interface{}, store S
 		// We used to make the call every 1sec (1 QPS), the goal here is to achieve ~98% traffic reduction when
 		// API server is not healthy. With these parameters, backoff will stop at [30,60) sec interval which is
 		// 0.22 QPS. If we don't backoff for 2min, assume API server is healthy and we reset the backoff.
-		backoffManager:         wait.NewExponentialBackoffManager(800*time.Millisecond, 30*time.Second, 2*time.Minute, 2.0, 1.0, reflectorClock),
-		initConnBackoffManager: wait.NewExponentialBackoffManager(800*time.Millisecond, 30*time.Second, 2*time.Minute, 2.0, 1.0, reflectorClock),
-		clock:                  reflectorClock,
-		watchErrorHandler:      WatchErrorHandler(DefaultWatchErrorHandler),
-		expectedType:           reflect.TypeOf(expectedType),
+		backoffManager:    wait.NewExponentialBackoffManager(800*time.Millisecond, 30*time.Second, 2*time.Minute, 2.0, 1.0, reflectorClock),
+		clock:             reflectorClock,
+		watchErrorHandler: WatchErrorHandler(DefaultWatchErrorHandler),
+		expectedType:      reflect.TypeOf(expectedType),
 	}
 
 	if r.name == "" {
@@ -425,7 +422,7 @@ func (r *Reflector) watch(w watch.Interface, stopCh <-chan struct{}, resyncerrc
 					select {
 					case <-stopCh:
 						return nil
-					case <-r.initConnBackoffManager.Backoff().C():
+					case <-r.backoffManager.Backoff().C():
 						continue
 					}
 				}
@@ -451,7 +448,7 @@ func (r *Reflector) watch(w watch.Interface, stopCh <-chan struct{}, resyncerrc
 					select {
 					case <-stopCh:
 						return nil
-					case <-r.initConnBackoffManager.Backoff().C():
+					case <-r.backoffManager.Backoff().C():
 						continue
 					}
 				case apierrors.IsInternalError(err) && retry.ShouldRetry():
@@ -604,7 +601,7 @@ func (r *Reflector) watchList(stopCh <-chan struct{}) (watch.Interface, error) {
 	isErrorRetriableWithSideEffectsFn := func(err error) bool {
 		if canRetry := isWatchErrorRetriable(err); canRetry {
 			klog.V(2).Infof("%s: watch-list of %v returned %v - backing off", r.name, r.typeDescription, err)
-			<-r.initConnBackoffManager.Backoff().C()
+			<-r.backoffManager.Backoff().C()
 			return true
 		}
 		if isExpiredError(err) || isTooLargeResourceVersionError(err) {
diff --git a/tools/cache/reflector_test.go b/tools/cache/reflector_test.go
index 2b7ee789..8b9457a7 100644
--- a/tools/cache/reflector_test.go
+++ b/tools/cache/reflector_test.go
@@ -411,12 +411,12 @@ func TestReflectorListAndWatchInitConnBackoff(t *testing.T) {
 					},
 				}
 				r := &Reflector{
-					name:                   "test-reflector",
-					listerWatcher:          lw,
-					store:                  NewFIFO(MetaNamespaceKeyFunc),
-					initConnBackoffManager: bm,
-					clock:                  fakeClock,
-					watchErrorHandler:      WatchErrorHandler(DefaultWatchErrorHandler),
+					name:              "test-reflector",
+					listerWatcher:     lw,
+					store:             NewFIFO(MetaNamespaceKeyFunc),
+					backoffManager:    bm,
+					clock:             fakeClock,
+					watchErrorHandler: WatchErrorHandler(DefaultWatchErrorHandler),
 				}
 				start := fakeClock.Now()
 				err := r.ListAndWatch(stopCh)
@@ -471,12 +471,12 @@ func TestBackoffOnTooManyRequests(t *testing.T) {
 	}
 
 	r := &Reflector{
-		name:                   "test-reflector",
-		listerWatcher:          lw,
-		store:                  NewFIFO(MetaNamespaceKeyFunc),
-		initConnBackoffManager: bm,
-		clock:                  clock,
-		watchErrorHandler:      WatchErrorHandler(DefaultWatchErrorHandler),
+		name:              "test-reflector",
+		listerWatcher:     lw,
+		store:             NewFIFO(MetaNamespaceKeyFunc),
+		backoffManager:    bm,
+		clock:             clock,
+		watchErrorHandler: WatchErrorHandler(DefaultWatchErrorHandler),
 	}
 
 	stopCh := make(chan struct{})
@@ -540,12 +540,12 @@ func TestRetryInternalError(t *testing.T) {
 		}
 
 		r := &Reflector{
-			name:                   "test-reflector",
-			listerWatcher:          lw,
-			store:                  NewFIFO(MetaNamespaceKeyFunc),
-			initConnBackoffManager: bm,
-			clock:                  fakeClock,
-			watchErrorHandler:      WatchErrorHandler(DefaultWatchErrorHandler),
+			name:              "test-reflector",
+			listerWatcher:     lw,
+			store:             NewFIFO(MetaNamespaceKeyFunc),
+			backoffManager:    bm,
+			clock:             fakeClock,
+			watchErrorHandler: WatchErrorHandler(DefaultWatchErrorHandler),
 		}
 
 		r.MaxInternalErrorRetryDuration = tc.maxInternalDuration