Fix behavior and documentation of max-retry-sleep flag (#1454)

* Fix DiffFiles utility function 1. It chunkifies DiffFiles utility function. This rationalizes the memory requirement of file-op of this utility. This utility earlier used to the read the complete files in memory, but now only a chunk is read and compared at a time. 2. Rename DiffFiles to AreFilesIdentical Renames file-op utility DiffFiles to AreFilesIdentical and changes it to return bool instead of int. * Copying logs on kokoro artifacts incase of integration test failure. (#1380) * adding print log file statement * failure test * testing error * undo testing changes * formating * fix comment * adding test error * small fix * small fix * testing * adding action artifacts * small fix * small fix * removing error statement * testing * testing * testing * undo changes * undo testing changes * fixing comments * Upgrading go version from 1.21.0 to 1.21.1 (#1401) * Upgrading go version to 1.21.1 * Temp changes to run perf/integration test * Revert "Temp changes to run perf/integration test" This reverts commit 18dd533. * Updating semantics docs (#1407) * Print stack-trace on crash For now, it just prints stack-trace whenever anyone calls logger.Fatalf(...). This needs to be enhanced further to include more scenarios (i.e. more sources of crashes). * Throw error in case of unexpected fields (#1416) Also, improved the doc for config-file. * Upgraded the fuse library (#1419) * add "create-empty-file: true" tests to run_tests_mounted_directory.sh (#1413) * Upgrade golang from 1.21.1 to 1.21.2 (#1431) Upgrade from golang 1.21.1 to 1.21.2 * Update yaml package version from v2 to v3 in integration tests (#1434) * update go yaml package version from v2 to v3 in integration tests * Empty-Commit * Passing gcsfuse flags from build.sh to mount the test bucket for perf… (#1430) * Passing gcsfuse flags from build.sh to mount the test bucket for performing list benchmarking * removing __pycache * small fix * small fix * unmount after fio tests * small fix * empty commit * testing kokoro perf tests * undo testing changes * adding upload flag * small fix * small fix * upload to upload_gs * unnecessary change * Creating big query tables for perf tests - 1 (#1444) * adding big query table setup * changes for small pr * indentation fix * removing unnecessary changes * adding start build time * changing description and flag names * fixing comment * updating flag help description * Formating changes * Update run_load_test_and_fetch_metrics.sh * undo unnecessay changes * empty commit --------- Co-authored-by: Nitin Garg <gargnitin@google.com> Co-authored-by: Prince Kumar <princer@google.com> Co-authored-by: Ayush Sethi <ayushsethi@google.com> Co-authored-by: Ashmeen Kaur <57195160+ashmeenkaur@users.noreply.github.com> Co-authored-by: Nitin Garg <113666283+gargnitingoogle@users.noreply.github.com>
GoogleCloudPlatform · Oct 27, 2023 · 176008d · 176008d
1 parent 4075a3f
commit 176008d
Show file tree

Hide file tree

Showing 7 changed files with 14 additions and 13 deletions.
diff --git a/flags.go b/flags.go
@@ -194,10 +194,9 @@ func newApp() (app *cli.App) {
 
 			cli.DurationFlag{
 				Name:  "max-retry-sleep",
-				Value: time.Minute,
-				Usage: "The maximum duration allowed to sleep in a retry loop with exponential backoff " +
-					"for failed requests to GCS backend. Once the backoff duration exceeds this limit, the retry stops." +
-					" The default is 1 minute. A value of 0 disables retries.",
+				Value: 30 * time.Second,
+				Usage: "The maximum duration allowed to sleep in a retry loop with exponential backoff for failed requests to GCS backend." +
+					" Once the backoff duration exceeds this limit, the retry continues with this specified maximum value.",
 			},
 
 			cli.IntFlag{
@@ -225,8 +224,8 @@ func newApp() (app *cli.App) {
 
 			cli.DurationFlag{
 				Name:  "max-retry-duration",
-				Value: 30 * time.Second,
-				Usage: "The operation will be retried till the value of max-retry-duration.",
+				Value: -1 * time.Second,
+				Usage: "This flag is currently unused.",
 			},
 
 			cli.Float64Flag{

diff --git a/flags_test.go b/flags_test.go
@@ -218,14 +218,16 @@ func (t *FlagsTest) Durations() {
 		"--stat-cache-ttl", "1m17s",
 		"--type-cache-ttl", "19ns",
 		"--http-client-timeout", "800ms",
-		"--max-retry-duration", "30s",
+		"--max-retry-duration", "-1s",
+		"--max-retry-sleep", "30s",
 	}
 
 	f := parseArgs(args)
 	ExpectEq(77*time.Second, f.StatCacheTTL)
 	ExpectEq(19*time.Nanosecond, f.TypeCacheTTL)
 	ExpectEq(800*time.Millisecond, f.HttpClientTimeout)
-	ExpectEq(30*time.Second, f.MaxRetryDuration)
+	ExpectEq(-1*time.Second, f.MaxRetryDuration)
+	ExpectEq(30*time.Second, f.MaxRetrySleep)
 }
 
 func (t *FlagsTest) Maps() {

diff --git a/internal/storage/storage_handle.go b/internal/storage/storage_handle.go
@@ -83,7 +83,7 @@ func NewStorageHandle(ctx context.Context, clientConfig storageutil.StorageClien
 	// https://github.com/googleapis/google-cloud-go/blob/main/storage/storage.go#L1953
 	sc.SetRetry(
 		storage.WithBackoff(gax.Backoff{
-			Max:        clientConfig.MaxRetryDuration,
+			Max:        clientConfig.MaxRetrySleep,
 			Multiplier: clientConfig.RetryMultiplier,
 		}),
 		storage.WithPolicy(storage.RetryAlways),

diff --git a/internal/storage/storageutil/client.go b/internal/storage/storageutil/client.go
@@ -32,7 +32,7 @@ type StorageClientConfig struct {
 	MaxConnsPerHost            int
 	MaxIdleConnsPerHost        int
 	HttpClientTimeout          time.Duration
-	MaxRetryDuration           time.Duration
+	MaxRetrySleep              time.Duration
 	RetryMultiplier            float64
 	UserAgent                  string
 	CustomEndpoint             *url.URL

diff --git a/internal/storage/storageutil/test_util.go b/internal/storage/storageutil/test_util.go
@@ -33,7 +33,7 @@ func GetDefaultStorageClientConfig() (clientConfig StorageClientConfig) {
 		MaxConnsPerHost:            10,
 		MaxIdleConnsPerHost:        100,
 		HttpClientTimeout:          800 * time.Millisecond,
-		MaxRetryDuration:           30 * time.Second,
+		MaxRetrySleep:              time.Minute,
 		RetryMultiplier:            2,
 		UserAgent:                  "gcsfuse/unknown (Go version go1.20-pre3 cl/474093167 +a813be86df) (GCP:gcsfuse)",
 		CustomEndpoint:             &url.URL{},

diff --git a/main.go b/main.go
@@ -87,7 +87,7 @@ func createStorageHandle(flags *flagStorage) (storageHandle storage.StorageHandl
 		MaxConnsPerHost:            flags.MaxConnsPerHost,
 		MaxIdleConnsPerHost:        flags.MaxIdleConnsPerHost,
 		HttpClientTimeout:          flags.HttpClientTimeout,
-		MaxRetryDuration:           flags.MaxRetryDuration,
+		MaxRetrySleep:              flags.MaxRetrySleep,
 		RetryMultiplier:            flags.RetryMultiplier,
 		UserAgent:                  getUserAgent(flags.AppName),
 		CustomEndpoint:             flags.CustomEndpoint,

diff --git a/main_test.go b/main_test.go
@@ -27,7 +27,7 @@ func (t *MainTest) TestCreateStorageHandle() {
 		MaxConnsPerHost:     5,
 		MaxIdleConnsPerHost: 100,
 		HttpClientTimeout:   5,
-		MaxRetryDuration:    7,
+		MaxRetrySleep:       7,
 		RetryMultiplier:     2,
 		AppName:             "app",
 		KeyFile:             "testdata/test_creds.json",