fix: admin retry logic

The previous fix wasn't correct. While looking at the transaction_manager.go
logic, it became clear that the intention is that retry.max should not
count the first attempt. This seems very reasonable so this fixes the test
and logic to reflect that. The original code would
fail this updated test because it didn't make the final required attempt.

Signed-off-by: Mark Hindess <[email protected]>

Author: hindessm

admin.go

@@ -207,7 +207,7 @@ func isErrNoController(err error) bool {
 // provided retryable func) up to the maximum number of tries permitted by
 // the admin client configuration
 func (ca *clusterAdmin) retryOnError(retryable func(error) bool, fn func() error) error {
-	for attemptsRemaining := ca.conf.Admin.Retry.Max; ; {
+	for attemptsRemaining := ca.conf.Admin.Retry.Max + 1; ; {
 		err := fn()
 		attemptsRemaining--
 		if err == nil || attemptsRemaining == 0 || !retryable(err) {

admin_test.go

@@ -1852,11 +1852,11 @@ func Test_retryOnError(t *testing.T) {
 		if err == nil {
 			t.Errorf("expected error but was nil")
 		}
-		if attempts != 3 {
-			t.Errorf("expected 3 attempts to have been made but was %d", attempts)
+		if attempts != 4 {
+			t.Errorf("expected 4 attempts to have been made but was %d", attempts)
 		}
-		if time.Since(startTime) >= 3*testBackoffTime {
-			t.Errorf("attempt+sleep+attempt+sleep+attempt should take less than 3 * backoff time")
+		if time.Since(startTime) >= 4*testBackoffTime {
+			t.Errorf("attempt+sleep+retry+sleep+retry+sleep+retry should take less than 4 * backoff time")
 		}
 	})
 }