Add a retryWaitTime option when enabling retry mechanism (#494)

Adding a new 'retryWaitTime' field that sets the number of seconds/milli-seconds to sleep between retries for upload, download, move, copy, delete and set/delete properties commands.
jfrog · Dec 29, 2021 · 7d6d021 · 7d6d021
1 parent cf6d660
commit 7d6d021
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 58 deletions.
diff --git a/access/manager.go b/access/manager.go
@@ -23,6 +23,7 @@ func New(config config.Config) (*AccessServicesManager, error) {
 		AppendPreRequestInterceptor(details.RunPreRequestFunctions).
 		SetContext(config.GetContext()).
 		SetRetries(config.GetHttpRetries()).
+		SetRetryWaitMilliSecs(config.GetHttpRetryWaitMilliSecs()).
 		Build()
 
 	return manager, err

diff --git a/artifactory/manager.go b/artifactory/manager.go
@@ -40,6 +40,7 @@ func NewWithProgress(config config.Config, progress ioutils.ProgressMgr) (Artifa
 		AppendPreRequestInterceptor(artDetails.RunPreRequestFunctions).
 		SetContext(config.GetContext()).
 		SetRetries(config.GetHttpRetries()).
+		SetRetryWaitMilliSecs(config.GetHttpRetryWaitMilliSecs()).
 		SetHttpClient(config.GetHttpClient()).
 		Build()
 	if err != nil {

diff --git a/artifactory/services/upload.go b/artifactory/services/upload.go
@@ -431,10 +431,10 @@ func (us *UploadService) uploadFileFromReader(getReaderFunc func() (io.Reader, e
 
 		if !checksumDeployed {
 			retryExecutor := clientutils.RetryExecutor{
-				MaxRetries:      us.client.GetHttpClient().GetRetries(),
-				RetriesInterval: 0,
-				ErrorMessage:    fmt.Sprintf("Failure occurred while uploading to %s", targetUrlWithProps),
-				LogMsgPrefix:    logMsgPrefix,
+				MaxRetries:               us.client.GetHttpClient().GetRetries(),
+				RetriesIntervalMilliSecs: us.client.GetHttpClient().GetRetryWaitTime(),
+				ErrorMessage:             fmt.Sprintf("Failure occurred while uploading to %s", targetUrlWithProps),
+				LogMsgPrefix:             logMsgPrefix,
 				ExecutionHandler: func() (bool, error) {
 					uploadZipReader, e := getReaderFunc()
 					if e != nil {

diff --git a/config/config.go b/config/config.go
@@ -18,20 +18,22 @@ type Config interface {
 	GetContext() context.Context
 	GetHttpTimeout() time.Duration
 	GetHttpRetries() int
+	GetHttpRetryWaitMilliSecs() int
 	GetHttpClient() *http.Client
 }
 
 type servicesConfig struct {
 	auth.ServiceDetails
-	certificatesPath string
-	dryRun           bool
-	threads          int
-	logger           log.Log
-	insecureTls      bool
-	ctx              context.Context
-	httpTimeout      time.Duration
-	httpRetries      int
-	httpClient       *http.Client
+	certificatesPath       string
+	dryRun                 bool
+	threads                int
+	logger                 log.Log
+	insecureTls            bool
+	ctx                    context.Context
+	httpTimeout            time.Duration
+	httpRetries            int
+	httpRetryWaitMilliSecs int
+	httpClient             *http.Client
 }
 
 func (config *servicesConfig) IsDryRun() bool {
@@ -70,6 +72,10 @@ func (config *servicesConfig) GetHttpRetries() int {
 	return config.httpRetries
 }
 
+func (config *servicesConfig) GetHttpRetryWaitMilliSecs() int {
+	return config.httpRetryWaitMilliSecs
+}
+
 func (config *servicesConfig) GetHttpClient() *http.Client {
 	return config.httpClient
 }
diff --git a/config/configbuilder.go b/config/configbuilder.go
@@ -13,19 +13,21 @@ func NewConfigBuilder() *servicesConfigBuilder {
 	configBuilder.threads = 3
 	configBuilder.httpTimeout = httpclient.DefaultHttpTimeout
 	configBuilder.httpRetries = 3
+	configBuilder.httpRetryWaitMilliSecs = 0
 	return configBuilder
 }
 
 type servicesConfigBuilder struct {
 	auth.ServiceDetails
-	certificatesPath string
-	threads          int
-	isDryRun         bool
-	insecureTls      bool
-	ctx              context.Context
-	httpTimeout      time.Duration
-	httpRetries      int
-	httpClient       *http.Client
+	certificatesPath       string
+	threads                int
+	isDryRun               bool
+	insecureTls            bool
+	ctx                    context.Context
+	httpTimeout            time.Duration
+	httpRetries            int
+	httpRetryWaitMilliSecs int
+	httpClient             *http.Client
 }
 
 func (builder *servicesConfigBuilder) SetServiceDetails(artDetails auth.ServiceDetails) *servicesConfigBuilder {
@@ -68,6 +70,11 @@ func (builder *servicesConfigBuilder) SetHttpRetries(httpRetries int) *servicesC
 	return builder
 }
 
+func (builder *servicesConfigBuilder) SetHttpRetryWaitMilliSecs(httpRetryWaitMilliSecs int) *servicesConfigBuilder {
+	builder.httpRetryWaitMilliSecs = httpRetryWaitMilliSecs
+	return builder
+}
+
 func (builder *servicesConfigBuilder) SetHttpClient(httpClient *http.Client) *servicesConfigBuilder {
 	builder.httpClient = httpClient
 	return builder
@@ -83,6 +90,7 @@ func (builder *servicesConfigBuilder) Build() (Config, error) {
 	c.ctx = builder.ctx
 	c.httpTimeout = builder.httpTimeout
 	c.httpRetries = builder.httpRetries
+	c.httpRetryWaitMilliSecs = builder.httpRetryWaitMilliSecs
 	c.httpClient = builder.httpClient
 	return c, nil
 }
diff --git a/distribution/manager.go b/distribution/manager.go
@@ -26,6 +26,7 @@ func New(config config.Config) (*DistributionServicesManager, error) {
 		AppendPreRequestInterceptor(details.RunPreRequestFunctions).
 		SetContext(config.GetContext()).
 		SetRetries(config.GetHttpRetries()).
+		SetRetryWaitMilliSecs(config.GetHttpRetryWaitMilliSecs()).
 		Build()
 	return manager, err
 }

diff --git a/http/httpclient/client.go b/http/httpclient/client.go
@@ -25,15 +25,20 @@ import (
 )
 
 type HttpClient struct {
-	client  *http.Client
-	ctx     context.Context
-	retries int
+	client             *http.Client
+	ctx                context.Context
+	retries            int
+	retryWaitMilliSecs int
 }
 
 func (jc *HttpClient) GetRetries() int {
 	return jc.retries
 }
 
+func (jc *HttpClient) GetRetryWaitTime() int {
+	return jc.retryWaitMilliSecs
+}
+
 func (jc *HttpClient) sendGetLeaveBodyOpen(url string, followRedirect bool, httpClientsDetails httputils.HttpClientDetails, logMsgPrefix string) (resp *http.Response, respBody []byte, redirectUrl string, err error) {
 	return jc.Send("GET", url, nil, followRedirect, false, httpClientsDetails, logMsgPrefix)
 }
@@ -92,10 +97,10 @@ func (jc *HttpClient) newRequest(method, url string, body io.Reader) (req *http.
 
 func (jc *HttpClient) Send(method, url string, content []byte, followRedirect, closeBody bool, httpClientsDetails httputils.HttpClientDetails, logMsgPrefix string) (resp *http.Response, respBody []byte, redirectUrl string, err error) {
 	retryExecutor := utils.RetryExecutor{
-		MaxRetries:      jc.retries,
-		RetriesInterval: 0,
-		LogMsgPrefix:    logMsgPrefix,
-		ErrorMessage:    fmt.Sprintf("Failure occurred while sending %s request to %s", method, url),
+		MaxRetries:               jc.retries,
+		RetriesIntervalMilliSecs: jc.retryWaitMilliSecs,
+		LogMsgPrefix:             logMsgPrefix,
+		ErrorMessage:             fmt.Sprintf("Failure occurred while sending %s request to %s", method, url),
 		ExecutionHandler: func() (bool, error) {
 			req, err := jc.createReq(method, url, content)
 			if err != nil {
@@ -191,10 +196,10 @@ func setRequestHeaders(httpClientsDetails httputils.HttpClientDetails, size int6
 func (jc *HttpClient) UploadFile(localPath, url, logMsgPrefix string, httpClientsDetails httputils.HttpClientDetails,
 	progress ioutils.ProgressMgr) (resp *http.Response, body []byte, err error) {
 	retryExecutor := utils.RetryExecutor{
-		MaxRetries:      jc.retries,
-		RetriesInterval: 0,
-		ErrorMessage:    fmt.Sprintf("Failure occurred while uploading to %s", url),
-		LogMsgPrefix:    logMsgPrefix,
+		MaxRetries:               jc.retries,
+		RetriesIntervalMilliSecs: jc.retryWaitMilliSecs,
+		ErrorMessage:             fmt.Sprintf("Failure occurred while uploading to %s", url),
+		LogMsgPrefix:             logMsgPrefix,
 		ExecutionHandler: func() (bool, error) {
 			resp, body, err = jc.doUploadFile(localPath, url, httpClientsDetails, progress)
 			if err != nil {
@@ -309,10 +314,10 @@ func (jc *HttpClient) DownloadFileNoRedirect(downloadPath, localPath, fileName s
 func (jc *HttpClient) downloadFile(downloadFileDetails *DownloadFileDetails, logMsgPrefix string, followRedirect bool,
 	httpClientsDetails httputils.HttpClientDetails, isExplode bool, progress ioutils.ProgressMgr) (resp *http.Response, redirectUrl string, err error) {
 	retryExecutor := utils.RetryExecutor{
-		MaxRetries:      jc.retries,
-		RetriesInterval: 0,
-		ErrorMessage:    fmt.Sprintf("Failure occurred while downloading %s", downloadFileDetails.DownloadPath),
-		LogMsgPrefix:    logMsgPrefix,
+		MaxRetries:               jc.retries,
+		RetriesIntervalMilliSecs: jc.retryWaitMilliSecs,
+		ErrorMessage:             fmt.Sprintf("Failure occurred while downloading %s", downloadFileDetails.DownloadPath),
+		LogMsgPrefix:             logMsgPrefix,
 		ExecutionHandler: func() (bool, error) {
 			resp, redirectUrl, err = jc.doDownloadFile(downloadFileDetails, logMsgPrefix, followRedirect, httpClientsDetails, isExplode, progress)
 			// In case followRedirect is 'false' and doDownloadFile did redirect, an error is returned and redirectUrl
@@ -598,10 +603,10 @@ func mergeChunks(chunksPaths []string, flags ConcurrentDownloadFlags) error {
 func (jc *HttpClient) downloadFileRange(flags ConcurrentDownloadFlags, start, end int64, currentSplit int, logMsgPrefix, chunkDownloadPath string,
 	httpClientsDetails httputils.HttpClientDetails, progress ioutils.ProgressMgr, progressId int) (fileName string, resp *http.Response, err error) {
 	retryExecutor := utils.RetryExecutor{
-		MaxRetries:      jc.retries,
-		RetriesInterval: 0,
-		ErrorMessage:    fmt.Sprintf("Failure occurred while downloading part %d of %s", currentSplit, flags.DownloadPath),
-		LogMsgPrefix:    fmt.Sprintf("%s[%s]: ", logMsgPrefix, strconv.Itoa(currentSplit)),
+		MaxRetries:               jc.retries,
+		RetriesIntervalMilliSecs: jc.retryWaitMilliSecs,
+		ErrorMessage:             fmt.Sprintf("Failure occurred while downloading part %d of %s", currentSplit, flags.DownloadPath),
+		LogMsgPrefix:             fmt.Sprintf("%s[%s]: ", logMsgPrefix, strconv.Itoa(currentSplit)),
 		ExecutionHandler: func() (bool, error) {
 			fileName, resp, err = jc.doDownloadFileRange(flags, start, end, currentSplit, logMsgPrefix, chunkDownloadPath, httpClientsDetails, progress, progressId)
 			if err != nil {

diff --git a/http/httpclient/clientBuilder.go b/http/httpclient/clientBuilder.go
@@ -27,6 +27,7 @@ type httpClientBuilder struct {
 	ctx                 context.Context
 	timeout             time.Duration
 	retries             int
+	retryWaitMilliSecs  int
 	httpClient          *http.Client
 }
 
@@ -70,6 +71,11 @@ func (builder *httpClientBuilder) SetRetries(retries int) *httpClientBuilder {
 	return builder
 }
 
+func (builder *httpClientBuilder) SetRetryWaitMilliSecs(retryWaitMilliSecs int) *httpClientBuilder {
+	builder.retryWaitMilliSecs = retryWaitMilliSecs
+	return builder
+}
+
 func (builder *httpClientBuilder) AddClientCertToTransport(transport *http.Transport) error {
 	if builder.clientCertPath != "" {
 		certificate, err := tls.LoadX509KeyPair(builder.clientCertPath, builder.clientCertKeyPath)
@@ -85,7 +91,7 @@ func (builder *httpClientBuilder) AddClientCertToTransport(transport *http.Trans
 func (builder *httpClientBuilder) Build() (*HttpClient, error) {
 	if builder.httpClient != nil {
 		// Using a custom http.Client, pass-though.
-		return &HttpClient{client: builder.httpClient, ctx: builder.ctx, retries: builder.retries}, nil
+		return &HttpClient{client: builder.httpClient, ctx: builder.ctx, retries: builder.retries, retryWaitMilliSecs: builder.retryWaitMilliSecs}, nil
 	}
 
 	var err error
@@ -101,7 +107,7 @@ func (builder *httpClientBuilder) Build() (*HttpClient, error) {
 		}
 	}
 	err = builder.AddClientCertToTransport(transport)
-	return &HttpClient{client: &http.Client{Transport: transport}, ctx: builder.ctx, retries: builder.retries}, err
+	return &HttpClient{client: &http.Client{Transport: transport}, ctx: builder.ctx, retries: builder.retries, retryWaitMilliSecs: builder.retryWaitMilliSecs}, err
 }
 
 func (builder *httpClientBuilder) createDefaultHttpTransport() *http.Transport {

diff --git a/http/jfroghttpclient/clientbuilder.go b/http/jfroghttpclient/clientbuilder.go
@@ -18,6 +18,7 @@ type jfrogHttpClientBuilder struct {
 	insecureTls            bool
 	ctx                    context.Context
 	retries                int
+	retryWaitTimMilliSecs  int
 	preRequestInterceptors []PreRequestInterceptorFunc
 	clientCertPath         string
 	clientCertKeyPath      string
@@ -55,6 +56,11 @@ func (builder *jfrogHttpClientBuilder) SetRetries(retries int) *jfrogHttpClientB
 	return builder
 }
 
+func (builder *jfrogHttpClientBuilder) SetRetryWaitMilliSecs(retryWaitMilliSecs int) *jfrogHttpClientBuilder {
+	builder.retryWaitTimMilliSecs = retryWaitMilliSecs
+	return builder
+}
+
 func (builder *jfrogHttpClientBuilder) AppendPreRequestInterceptor(interceptor PreRequestInterceptorFunc) *jfrogHttpClientBuilder {
 	builder.preRequestInterceptors = append(builder.preRequestInterceptors, interceptor)
 	return builder
@@ -80,6 +86,7 @@ func (builder *jfrogHttpClientBuilder) Build() (rtHttpClient *JfrogHttpClient, e
 		SetContext(builder.ctx).
 		SetTimeout(builder.timeout).
 		SetRetries(builder.retries).
+		SetRetryWaitMilliSecs(builder.retryWaitTimMilliSecs).
 		SetHttpClient(builder.httpClient).
 		Build()
 	return

diff --git a/pipelines/manager.go b/pipelines/manager.go
@@ -23,6 +23,7 @@ func New(config config.Config) (*PipelinesServicesManager, error) {
 		AppendPreRequestInterceptor(details.RunPreRequestFunctions).
 		SetContext(config.GetContext()).
 		SetRetries(config.GetHttpRetries()).
+		SetRetryWaitMilliSecs(config.GetHttpRetryWaitMilliSecs()).
 		Build()
 	return manager, err
 }

diff --git a/tests/utils_test.go b/tests/utils_test.go
@@ -821,10 +821,11 @@ func createRepoConfigValidationFunc(repoKey string, expectedConfig interface{})
 
 func validateRepoConfig(t *testing.T, repoKey string, params interface{}) {
 	retryExecutor := &clientutils.RetryExecutor{
-		MaxRetries:       12,
-		RetriesInterval:  10,
-		ErrorMessage:     "Waiting for Artifactory to evaluate repository operation...",
-		ExecutionHandler: createRepoConfigValidationFunc(repoKey, params),
+		MaxRetries: 12,
+		// RetriesIntervalMilliSecs in milliseconds
+		RetriesIntervalMilliSecs: 10 * 1000,
+		ErrorMessage:             "Waiting for Artifactory to evaluate repository operation...",
+		ExecutionHandler:         createRepoConfigValidationFunc(repoKey, params),
 	}
 	err := retryExecutor.Execute()
 	assert.NoError(t, err)

diff --git a/utils/retryexecutor.go b/utils/retryexecutor.go
@@ -3,6 +3,7 @@ package utils
 import (
 	"fmt"
 	"github.com/jfrog/jfrog-client-go/utils/log"
+	"strconv"
 	"time"
 )
 
@@ -12,8 +13,8 @@ type RetryExecutor struct {
 	// The amount of retries to perform.
 	MaxRetries int
 
-	// Number of seconds to sleep between retries.
-	RetriesInterval int
+	// Number of milliseconds to sleep between retries.
+	RetriesIntervalMilliSecs int
 
 	// Message to display when retrying.
 	ErrorMessage string
@@ -38,9 +39,10 @@ func (runner *RetryExecutor) Execute() error {
 		}
 
 		log.Warn(runner.getLogRetryMessage(i, err))
-		// Going to sleep for RetryInterval seconds
-		if runner.RetriesInterval > 0 && i < runner.MaxRetries {
-			time.Sleep(time.Second * time.Duration(runner.RetriesInterval))
+		// Going to sleep for RetryInterval milliseconds
+		if runner.RetriesIntervalMilliSecs > 0 && i < runner.MaxRetries {
+			log.Info("Waiting ", strconv.Itoa(runner.RetriesIntervalMilliSecs), "ms before trying again")
+			time.Sleep(time.Millisecond * time.Duration(runner.RetriesIntervalMilliSecs))
 		}
 	}
 

diff --git a/utils/retryexecutor_test.go b/utils/retryexecutor_test.go
@@ -11,11 +11,10 @@ func TestRetryExecutorSuccess(t *testing.T) {
 	retriesToPerform := 10
 	breakRetriesAt := 4
 	runCount := 0
-
 	executor := RetryExecutor{
-		MaxRetries:      retriesToPerform,
-		RetriesInterval: 0,
-		ErrorMessage:    "Testing RetryExecutor",
+		MaxRetries:               retriesToPerform,
+		RetriesIntervalMilliSecs: 0,
+		ErrorMessage:             "Testing RetryExecutor",
 		ExecutionHandler: func() (bool, error) {
 			runCount++
 			if runCount == breakRetriesAt {
@@ -37,9 +36,9 @@ func TestRetryExecutorFail(t *testing.T) {
 	runCount := 0
 
 	executor := RetryExecutor{
-		MaxRetries:      retriesToPerform,
-		RetriesInterval: 0,
-		ErrorMessage:    "Testing RetryExecutor",
+		MaxRetries:               retriesToPerform,
+		RetriesIntervalMilliSecs: 0,
+		ErrorMessage:             "Testing RetryExecutor",
 		ExecutionHandler: func() (bool, error) {
 			runCount++
 			return true, nil

diff --git a/xray/manager.go b/xray/manager.go
@@ -27,6 +27,7 @@ func New(config config.Config) (*XrayServicesManager, error) {
 		SetClientCertKeyPath(details.GetClientCertKeyPath()).
 		AppendPreRequestInterceptor(details.RunPreRequestFunctions).
 		SetRetries(config.GetHttpRetries()).
+		SetRetryWaitMilliSecs(config.GetHttpRetryWaitMilliSecs()).
 		Build()
 	return manager, err
 }