Merge pull request #2070 from GoogleCloudPlatform/master

Merge master into CLI_config_parity_release
GoogleCloudPlatform · Jun 27, 2024 · 2a6fb30 · 2a6fb30
2 parents ebe610c + b8aa8dc
commit 2a6fb30
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 5 deletions.
diff --git a/cfg/config.go b/cfg/config.go
@@ -20,6 +20,7 @@ import (
 	"net/url"
 	"time"
 
+	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 )
@@ -560,7 +561,7 @@ func BindFlags(flagSet *pflag.FlagSet) error {
 		return err
 	}
 
-	flagSet.IntP("max-parallel-downloads", "", -1, "Sets an uber limit of number of concurrent file download requests that are made across all files.")
+	flagSet.IntP("max-parallel-downloads", "", config.DefaultMaxParallelDownloads(), "Sets an uber limit of number of concurrent file download requests that are made across all files.")
 
 	err = viper.BindPFlag("file-cache.max-parallel-downloads", flagSet.Lookup("max-parallel-downloads"))
 	if err != nil {

diff --git a/cmd/root_test.go b/cmd/root_test.go
@@ -48,6 +48,20 @@ func TestValidConfig(t *testing.T) {
 	assert.Nil(t, cmd.Execute())
 }
 
+func TestDefaultMaxParallelDownloads(t *testing.T) {
+	var actual cfg.Config
+	cmd, err := NewRootCmd(func(c cfg.Config) error {
+		actual = c
+		return nil
+	})
+	require.Nil(t, err)
+	cmd.SetArgs([]string{"abc", "pqr"})
+
+	if assert.Nil(t, cmd.Execute()) {
+		assert.LessOrEqual(t, int64(16), actual.FileCache.MaxParallelDownloads)
+	}
+}
+
 func TestTooManyCobraArgs(t *testing.T) {
 	tests := []struct {
 		name        string

diff --git a/internal/cache/file/downloader/jm_parallel_downloads_test.go b/internal/cache/file/downloader/jm_parallel_downloads_test.go
@@ -127,6 +127,7 @@ func TestParallelDownloads(t *testing.T) {
 	}
 	for _, tc := range tbl {
 		t.Run(tc.name, func(t *testing.T) {
+			tc := tc
 			t.Parallel()
 			cache, cacheDir := configureCache(t, 2*tc.objectSize)
 			storageHandle := configureFakeStorage(t)

diff --git a/internal/config/config_util.go b/internal/config/config_util.go
@@ -17,6 +17,7 @@ package config
 import (
 	"fmt"
 	"math"
+	"runtime"
 	"time"
 )
 
@@ -112,3 +113,7 @@ func ListCacheTtlSecsToDuration(secs int64) time.Duration {
 
 	return time.Duration(secs * int64(time.Second))
 }
+
+func DefaultMaxParallelDownloads() int {
+	return max(16, 2*runtime.NumCPU())
+}
diff --git a/internal/config/config_util_test.go b/internal/config/config_util_test.go
@@ -267,3 +267,7 @@ func Test_ListCacheTtlSecsToDuration_InvalidCall(t *testing.T) {
 	// Calling with invalid argument to trigger panic.
 	ListCacheTtlSecsToDuration(-3)
 }
+
+func Test_DefaultMaxParallelDownloads(t *testing.T) {
+	assert.GreaterOrEqual(t, DefaultMaxParallelDownloads(), 16)
+}
diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go
@@ -63,7 +63,6 @@ const (
 	DefaultEnableParallelDownloads  = false
 	DefaultDownloadChunkSizeMB      = 25
 	DefaultParallelDownloadsPerFile = 10
-	DefaultMaxParallelDownloads     = -1
 )
 
 type WriteConfig struct {
@@ -187,7 +186,7 @@ func NewMountConfig() *MountConfig {
 		MaxSizeMB:                DefaultFileCacheMaxSizeMB,
 		EnableParallelDownloads:  DefaultEnableParallelDownloads,
 		ParallelDownloadsPerFile: DefaultParallelDownloadsPerFile,
-		MaxParallelDownloads:     DefaultMaxParallelDownloads,
+		MaxParallelDownloads:     DefaultMaxParallelDownloads(),
 		DownloadChunkSizeMB:      DefaultDownloadChunkSizeMB,
 		EnableCRC:                DefaultEnableCRC,
 	}

diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go
@@ -44,7 +44,7 @@ func validateDefaultConfig(t *testing.T, mountConfig *MountConfig) {
 	assert.False(t, mountConfig.FileCacheConfig.CacheFileForRangeRead)
 	assert.False(t, mountConfig.FileCacheConfig.EnableParallelDownloads)
 	assert.Equal(t, 10, mountConfig.FileCacheConfig.ParallelDownloadsPerFile)
-	assert.Equal(t, -1, mountConfig.FileCacheConfig.MaxParallelDownloads)
+	assert.GreaterOrEqual(t, mountConfig.FileCacheConfig.MaxParallelDownloads, 16)
 	assert.Equal(t, 25, mountConfig.FileCacheConfig.DownloadChunkSizeMB)
 	assert.True(t, mountConfig.FileCacheConfig.EnableCRC)
 	assert.Equal(t, 1, mountConfig.GCSConnection.GRPCConnPoolSize)

diff --git a/tools/config-gen/config.tpl b/tools/config-gen/config.tpl
@@ -20,6 +20,7 @@ import (
 	"time"
 	"net/url"
 
+	"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 )

diff --git a/tools/config-gen/params.yaml b/tools/config-gen/params.yaml
@@ -451,7 +451,7 @@
   config-path: "file-cache.max-parallel-downloads"
   type: "int"
   usage: "Sets an uber limit of number of concurrent file download requests that are made across all files."
-  default: "-1"
+  default: "config.DefaultMaxParallelDownloads()"
 
 - flag-name: "download-chunk-size-mb"
   config-path: "file-cache.download-chunk-size-mb"