fix: Utilize a backoff for git retried operations (argoproj#7395)

* fix: git retry duration Signed-off-by: May Zhang <[email protected]> * fix: adjust default values Signed-off-by: May Zhang <[email protected]> * fix: adjust default values Signed-off-by: May Zhang <[email protected]>
tkasuz · Oct 8, 2021 · 3c874ae · 3c874ae
1 parent 4dd570e
commit 3c874ae
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 1 deletion.
diff --git a/common/common.go b/common/common.go
@@ -147,6 +147,12 @@ const (
 	EnvVarTLSDataPath = "ARGOCD_TLS_DATA_PATH"
 	// Specifies number of git remote operations attempts count
 	EnvGitAttemptsCount = "ARGOCD_GIT_ATTEMPTS_COUNT"
+	// Specifices max duration of git remote operation retry
+	EnvGitRetryMaxDuration = "ARGOCD_GIT_RETRY_MAX_DURATION"
+	// Specifies duration of git remote operation retry
+	EnvGitRetryDuration = "ARGOCD_GIT_RETRY_DURATION"
+	// Specifies fator of git remote operation retry
+	EnvGitRetryFactor = "ARGOCD_GIT_RETRY_FACTOR"
 	// Overrides git submodule support, true by default
 	EnvGitSubmoduleEnabled = "ARGOCD_GIT_MODULES_ENABLED"
 	// EnvGnuPGHome is the path to ArgoCD's GnuPG keyring for signature verification
@@ -187,6 +193,12 @@ const (
 	CacheVersion = "1.8.3"
 )
 
+const (
+	DefaultGitRetryMaxDuration time.Duration = time.Second * 5        // 5s
+	DefaultGitRetryDuration    time.Duration = time.Millisecond * 250 // 0.25s
+	DefaultGitRetryFactor                    = int64(2)
+)
+
 // GetGnuPGHomePath retrieves the path to use for GnuPG home directory, which is either taken from GNUPGHOME environment or a default value
 func GetGnuPGHomePath() string {
 	if gnuPgHome := os.Getenv(EnvGnuPGHome); gnuPgHome == "" {

diff --git a/util/env/env.go b/util/env/env.go
@@ -37,6 +37,33 @@ func ParseNumFromEnv(env string, defaultValue, min, max int) int {
 	return num
 }
 
+// Helper function to parse a int64 from an environment variable. Returns a
+// default if env is not set, is not parseable to a number, exceeds max (if
+// max is greater than 0) or is less than min.
+//
+// nolint:unparam
+func ParseInt64FromEnv(env string, defaultValue, min, max int64) int64 {
+	str := os.Getenv(env)
+	if str == "" {
+		return defaultValue
+	}
+
+	num, err := strconv.ParseInt(str, 10, 64)
+	if err != nil {
+		log.Warnf("Could not parse '%s' as a int64 from environment %s", str, env)
+		return defaultValue
+	}
+	if num < min {
+		log.Warnf("Value in %s is %d, which is less than minimum %d allowed", env, num, min)
+		return defaultValue
+	}
+	if num > max {
+		log.Warnf("Value in %s is %d, which is greater than maximum %d allowed", env, num, max)
+		return defaultValue
+	}
+	return num
+}
+
 // Helper function to parse a time duration from an environment variable. Returns a
 // default if env is not set, is not parseable to a duration, exceeds max (if
 // max is greater than 0) or is less than min.

diff --git a/util/git/client.go b/util/git/client.go
@@ -24,9 +24,12 @@ import (
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/knownhosts"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	utilnet "k8s.io/apimachinery/pkg/util/net"
 
 	"github.com/argoproj/argo-cd/v2/common"
 	certutil "github.com/argoproj/argo-cd/v2/util/cert"
+	"github.com/argoproj/argo-cd/v2/util/env"
 	executil "github.com/argoproj/argo-cd/v2/util/exec"
 	"github.com/argoproj/argo-cd/v2/util/proxy"
 )
@@ -94,6 +97,9 @@ type nativeGitClient struct {
 
 var (
 	maxAttemptsCount = 1
+	maxRetryDuration time.Duration
+	retryDuration    time.Duration
+	factor           int64
 )
 
 func init() {
@@ -104,6 +110,11 @@ func init() {
 			maxAttemptsCount = int(math.Max(float64(cnt), 1))
 		}
 	}
+
+	maxRetryDuration = env.ParseDurationFromEnv(common.EnvGitRetryMaxDuration, common.DefaultGitRetryMaxDuration, 0, math.MaxInt64)
+	retryDuration = env.ParseDurationFromEnv(common.EnvGitRetryDuration, common.DefaultGitRetryDuration, 0, math.MaxInt64)
+	factor = env.ParseInt64FromEnv(common.EnvGitRetryFactor, common.DefaultGitRetryFactor, 0, math.MaxInt64)
+
 }
 
 type ClientOpts func(c *nativeGitClient)
@@ -470,8 +481,19 @@ func (m *nativeGitClient) LsRefs() (*Refs, error) {
 // repository locally cloned.
 func (m *nativeGitClient) LsRemote(revision string) (res string, err error) {
 	for attempt := 0; attempt < maxAttemptsCount; attempt++ {
-		if res, err = m.lsRemote(revision); err == nil {
+		res, err = m.lsRemote(revision)
+		if err == nil {
 			return
+		} else if apierrors.IsInternalError(err) || apierrors.IsTimeout(err) || apierrors.IsServerTimeout(err) ||
+			apierrors.IsTooManyRequests(err) || utilnet.IsProbableEOF(err) || utilnet.IsConnectionReset(err) {
+			// Formula: timeToWait = duration * factor^retry_number
+			// Note that timeToWait should equal to duration for the first retry attempt.
+			// When timeToWait is more than maxDuration retry should be performed at maxDuration.
+			timeToWait := float64(retryDuration) * (math.Pow(float64(factor), float64(attempt)))
+			if maxRetryDuration > 0 {
+				timeToWait = math.Min(float64(maxRetryDuration), timeToWait)
+			}
+			time.Sleep(time.Duration(timeToWait))
 		}
 	}
 	return