rename disable o direct to enable-o-direct (#2417)

* rename disable o direct to use o direct

* renaming to enable-o-direct

* fix

cfg/config.go

@@ -74,12 +74,12 @@ type DebugConfig struct {
 type FileCacheConfig struct {
 	CacheFileForRangeRead bool `yaml:"cache-file-for-range-read,omitempty" json:"cache-file-for-range-read,omitempty"`
 
-	DisableODirect bool `yaml:"disable-o-direct,omitempty" json:"disable-o-direct,omitempty"`
-
 	DownloadChunkSizeMb int64 `yaml:"download-chunk-size-mb,omitempty" json:"download-chunk-size-mb,omitempty"`
 
 	EnableCrc bool `yaml:"enable-crc,omitempty" json:"enable-crc,omitempty"`
 
+	EnableODirect bool `yaml:"enable-o-direct,omitempty" json:"enable-o-direct,omitempty"`
+
 	EnableParallelDownloads bool `yaml:"enable-parallel-downloads,omitempty" json:"enable-parallel-downloads,omitempty"`
 
 	MaxParallelDownloads int64 `yaml:"max-parallel-downloads,omitempty" json:"max-parallel-downloads,omitempty"`
@@ -319,16 +319,6 @@ func BindFlags(v *viper.Viper, flagSet *pflag.FlagSet) error {
 		return err
 	}
 
-	flagSet.BoolP("disable-o-direct", "", true, "Whether to disable using O_DIRECT while writing to file-cache in case of parallel downloads.")
-
-	if err := flagSet.MarkHidden("disable-o-direct"); err != nil {
-		return err
-	}
-
-	if err := v.BindPFlag("file-cache.disable-o-direct", flagSet.Lookup("disable-o-direct")); err != nil {
-		return err
-	}
-
 	flagSet.BoolP("disable-parallel-dirops", "", false, "Specifies whether to allow parallel dir operations (lookups and readers)")
 
 	if err := flagSet.MarkHidden("disable-parallel-dirops"); err != nil {
@@ -369,6 +359,16 @@ func BindFlags(v *viper.Viper, flagSet *pflag.FlagSet) error {
 		return err
 	}
 
+	flagSet.BoolP("enable-o-direct", "", false, "Whether to use O_DIRECT while writing to file-cache in case of parallel downloads.")
+
+	if err := flagSet.MarkHidden("enable-o-direct"); err != nil {
+		return err
+	}
+
+	if err := v.BindPFlag("file-cache.enable-o-direct", flagSet.Lookup("enable-o-direct")); err != nil {
+		return err
+	}
+
 	flagSet.BoolP("enable-parallel-downloads", "", false, "Enable parallel downloads.")
 
 	if err := v.BindPFlag("file-cache.enable-parallel-downloads", flagSet.Lookup("enable-parallel-downloads")); err != nil {

cfg/params.yaml

@@ -83,13 +83,6 @@
   usage: "Whether to cache file for range reads."
   default: false
 
-- config-path: "file-cache.disable-o-direct"
-  flag-name: "disable-o-direct"
-  type: "bool"
-  usage: "Whether to disable using O_DIRECT while writing to file-cache in case of parallel downloads."
-  default: "true"
-  hide-flag: true
-
 - config-path: "file-cache.download-chunk-size-mb"
   flag-name: "download-chunk-size-mb"
   type: "int"
@@ -102,6 +95,13 @@
   usage: "Performs CRC to ensure that file is correctly downloaded into cache."
   default: false
 
+- config-path: "file-cache.enable-o-direct"
+  flag-name: "enable-o-direct"
+  type: "bool"
+  usage: "Whether to use O_DIRECT while writing to file-cache in case of parallel downloads."
+  default: "false"
+  hide-flag: true
+
 - config-path: "file-cache.enable-parallel-downloads"
   flag-name: "enable-parallel-downloads"
   type: "bool"

cfg/validate_test.go

@@ -39,7 +39,7 @@ func validFileCacheConfig(t *testing.T) FileCacheConfig {
 		MaxSizeMb:                -1,
 		ParallelDownloadsPerFile: 16,
 		WriteBufferSize:          4 * 1024 * 1024,
-		DisableODirect:           true,
+		EnableODirect:            true,
 	}
 }
 

cmd/config_validation_test.go

@@ -61,7 +61,7 @@ func defaultFileCacheConfig(t *testing.T) cfg.FileCacheConfig {
 		MaxSizeMb:                -1,
 		ParallelDownloadsPerFile: 16,
 		WriteBufferSize:          4 * 1024 * 1024,
-		DisableODirect:           true,
+		EnableODirect:            false,
 	}
 }
 
@@ -320,7 +320,7 @@ func TestValidateConfigFile_FileCacheConfigSuccessful(t *testing.T) {
 					MaxSizeMb:                40,
 					ParallelDownloadsPerFile: 10,
 					WriteBufferSize:          8192,
-					DisableODirect:           true,
+					EnableODirect:            true,
 				},
 			},
 		},

cmd/root_test.go

@@ -240,7 +240,7 @@ func TestArgsParsing_FileCacheFlags(t *testing.T) {
 	}{
 		{
 			name: "Test file cache flags.",
-			args: []string{"gcsfuse", "--cache-file-for-range-read", "--download-chunk-size-mb=20", "--enable-crc", "--enable-parallel-downloads", "--max-parallel-downloads=40", "--file-cache-max-size-mb=100", "--parallel-downloads-per-file=2", "--disable-o-direct=false", "abc", "pqr"},
+			args: []string{"gcsfuse", "--cache-file-for-range-read", "--download-chunk-size-mb=20", "--enable-crc", "--enable-parallel-downloads", "--max-parallel-downloads=40", "--file-cache-max-size-mb=100", "--parallel-downloads-per-file=2", "--enable-o-direct=false", "abc", "pqr"},
 			expectedConfig: &cfg.Config{
 				FileCache: cfg.FileCacheConfig{
 					CacheFileForRangeRead:    true,
@@ -251,7 +251,7 @@ func TestArgsParsing_FileCacheFlags(t *testing.T) {
 					MaxSizeMb:                100,
 					ParallelDownloadsPerFile: 2,
 					WriteBufferSize:          4 * 1024 * 1024,
-					DisableODirect:           false,
+					EnableODirect:            false,
 				},
 			},
 		},
@@ -268,7 +268,7 @@ func TestArgsParsing_FileCacheFlags(t *testing.T) {
 					MaxSizeMb:                -1,
 					ParallelDownloadsPerFile: 16,
 					WriteBufferSize:          4 * 1024 * 1024,
-					DisableODirect:           true,
+					EnableODirect:            false,
 				},
 			},
 		},

cmd/testdata/valid_config.yaml

@@ -10,7 +10,7 @@ file-cache:
   max-size-mb: 40
   parallel-downloads-per-file: 10
   write-buffer-size: 8192
-  disable-o-direct: true
+  enable-o-direct: true
 gcs-auth:
   anonymous-access: true
   key-file: "~/key.file"

internal/cache/file/downloader/jm_parallel_downloads_test.go

@@ -99,7 +99,7 @@ func TestParallelDownloads(t *testing.T) {
 		maxParallelDownloads     int64
 		downloadOffset           int64
 		subscribedOffset         int64
-		disableODirect           bool
+		enableODirect            bool
 	}{
 		{
 			name:                     "download the entire object when object size > no of goroutines * readReqSize",
@@ -109,6 +109,7 @@ func TestParallelDownloads(t *testing.T) {
 			maxParallelDownloads:     3,
 			subscribedOffset:         7,
 			downloadOffset:           10,
+			enableODirect:            true,
 		},
 		{
 			name:                     "download only upto the object size",
@@ -118,6 +119,7 @@ func TestParallelDownloads(t *testing.T) {
 			maxParallelDownloads:     3,
 			subscribedOffset:         7,
 			downloadOffset:           10,
+			enableODirect:            true,
 		},
 		{
 			name:                     "download the entire object with O_DIRECT disabled.",
@@ -127,7 +129,7 @@ func TestParallelDownloads(t *testing.T) {
 			maxParallelDownloads:     3,
 			subscribedOffset:         7,
 			downloadOffset:           10,
-			disableODirect:           true,
+			enableODirect:            false,
 		},
 	}
 	for _, tc := range tbl {
@@ -144,7 +146,7 @@ func TestParallelDownloads(t *testing.T) {
 				DownloadChunkSizeMb:      tc.readReqSize, EnableCrc: true,
 				MaxParallelDownloads: tc.maxParallelDownloads,
 				WriteBufferSize:      4 * 1024 * 1024,
-				DisableODirect:       tc.disableODirect,
+				EnableODirect:        tc.enableODirect,
 			}
 			jm := NewJobManager(cache, util.DefaultFilePerm, util.DefaultDirPerm, cacheDir, 2, fileCacheConfig)
 			job := jm.CreateJobIfNotExists(&minObj, bucket)

internal/cache/file/downloader/job.go

@@ -382,7 +382,7 @@ func (job *Job) createCacheFile() (*os.File, error) {
 	var err error
 	// Try using O_DIRECT while opening file when parallel downloads are enabled
 	// and O_DIRECT use is not disabled.
-	if job.fileCacheConfig.EnableParallelDownloads && !job.fileCacheConfig.DisableODirect {
+	if job.fileCacheConfig.EnableParallelDownloads && !job.fileCacheConfig.EnableODirect {
 		cacheFile, err = cacheutil.CreateFile(job.fileSpec, openFileFlags|syscall.O_DIRECT)
 		if errors.Is(err, fs.ErrInvalid) || errors.Is(err, syscall.EINVAL) {
 			logger.Warnf("downloadObjectAsync: failure in opening file with O_DIRECT, falling back to without O_DIRECT")

internal/cache/file/downloader/job_test.go

@@ -1046,7 +1046,7 @@ func (dt *downloaderTest) Test_createCacheFile_WhenParallelDownloads() {
 func (dt *downloaderTest) Test_createCacheFile_WhenParallelDownloadsEnabledAndODirectDisabled() {
 	//Arrange - initJobTest is being called in setup of downloader.go
 	dt.job.fileCacheConfig.EnableParallelDownloads = true
-	dt.job.fileCacheConfig.DisableODirect = true
+	dt.job.fileCacheConfig.EnableODirect = false
 
 	cacheFile, err := dt.job.createCacheFile()
 

internal/cache/file/downloader/parallel_downloads_job.go

@@ -63,7 +63,7 @@ func (job *Job) downloadRange(ctx context.Context, dstWriter io.Writer, start, e
 	monitor.CaptureGCSReadMetrics(ctx, util.Parallel, end-start)
 
 	// Use of memory aligned buffer is not required if use of O_DIRECT is disabled.
-	if job.fileCacheConfig.DisableODirect {
+	if job.fileCacheConfig.EnableODirect {
 		_, err = io.CopyN(dstWriter, newReader, end-start)
 	} else {
 		_, err = cacheutil.CopyUsingMemoryAlignedBuffer(ctx, newReader, dstWriter, end-start,

internal/config/mount_config.go

@@ -38,7 +38,7 @@ const (
 	DefaultDownloadChunkSizeMB      = 50
 	DefaultParallelDownloadsPerFile = 16
 	DefaultWriteBufferSize          = int64(4 * util.MiB)
-	DefaultDisableODirect           = true
+	DefaultEnableODirect            = false
 
 	// DefaultTypeCacheMaxSizeMB is the default value of type-cache max-size for every directory in MiBs.
 	// The value is set at the size needed for about 21k type-cache entries,
@@ -95,7 +95,7 @@ type FileCacheConfig struct {
 	DownloadChunkSizeMB      int   `yaml:"download-chunk-size-mb,omitempty"`
 	EnableCRC                bool  `yaml:"enable-crc"`
 	WriteBufferSize          int64 `yaml:"write-buffer-size,omitempty"`
-	DisableODirect           bool  `yaml:"disable-o-direct"`
+	EnableODirect            bool  `yaml:"enable-o-direct"`
 }
 
 type MetadataCacheConfig struct {
@@ -175,7 +175,7 @@ func NewMountConfig() *MountConfig {
 		DownloadChunkSizeMB:      DefaultDownloadChunkSizeMB,
 		EnableCRC:                DefaultEnableCRC,
 		WriteBufferSize:          DefaultWriteBufferSize,
-		DisableODirect:           DefaultDisableODirect,
+		EnableODirect:            DefaultEnableODirect,
 	}
 	mountConfig.MetadataCacheConfig = MetadataCacheConfig{
 		TtlInSeconds:       cfg.TtlInSecsUnsetSentinel,

internal/config/testdata/valid_config.yaml

@@ -18,7 +18,7 @@ file-cache:
   download-chunk-size-mb: 100
   enable-crc: false
   write-buffer-size: 8192
-  disable-o-direct: false
+  enable-o-direct: false
 metadata-cache:
   ttl-secs: 5
   type-cache-max-size-mb: 1

internal/config/yaml_parser_test.go

@@ -49,7 +49,7 @@ func validateDefaultConfig(t *testing.T, mountConfig *MountConfig) {
 	assert.Equal(t, 50, mountConfig.FileCacheConfig.DownloadChunkSizeMB)
 	assert.False(t, mountConfig.FileCacheConfig.EnableCRC)
 	assert.Equal(t, int64(4*1024*1024), mountConfig.FileCacheConfig.WriteBufferSize)
-	assert.Equal(t, true, mountConfig.FileCacheConfig.DisableODirect)
+	assert.Equal(t, false, mountConfig.FileCacheConfig.EnableODirect)
 	assert.Equal(t, 1, mountConfig.GCSConnection.GRPCConnPoolSize)
 	assert.False(t, mountConfig.GCSAuth.AnonymousAccess)
 	assert.False(t, bool(mountConfig.EnableHNS))
@@ -134,7 +134,7 @@ func (t *YamlParserTest) TestReadConfigFile_ValidConfig() {
 	assert.Equal(t.T(), 100, mountConfig.DownloadChunkSizeMB)
 	assert.False(t.T(), mountConfig.FileCacheConfig.EnableCRC)
 	assert.Equal(t.T(), int64(8192), mountConfig.FileCacheConfig.WriteBufferSize)
-	assert.Equal(t.T(), false, mountConfig.FileCacheConfig.DisableODirect)
+	assert.Equal(t.T(), false, mountConfig.FileCacheConfig.EnableODirect)
 
 	// gcs-retries
 	assert.Equal(t.T(), int64(6), mountConfig.GCSRetries.MaxRetryAttempts)

tools/integration_tests/read_cache/cache_file_for_range_read_false_test.go

@@ -153,7 +153,7 @@ func TestCacheFileForRangeReadFalseTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileName,
 			enableParallelDownloads: false,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -162,7 +162,7 @@ func TestCacheFileForRangeReadFalseTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileName,
 			enableParallelDownloads: false,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            ramCacheDir,
 		},
 	}
@@ -184,7 +184,7 @@ func TestCacheFileForRangeReadFalseTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -193,7 +193,7 @@ func TestCacheFileForRangeReadFalseTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            ramCacheDir,
 		},
 		{
@@ -202,7 +202,7 @@ func TestCacheFileForRangeReadFalseTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          false,
+			enableODirect:           true,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -211,7 +211,7 @@ func TestCacheFileForRangeReadFalseTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          false,
+			enableODirect:           true,
 			cacheDirPath:            ramCacheDir,
 		},
 	}

tools/integration_tests/read_cache/cache_file_for_range_read_true_test.go

@@ -105,7 +105,7 @@ func TestCacheFileForRangeReadTrueTest(t *testing.T) {
 			cacheFileForRangeRead:   true,
 			fileName:                configFileName,
 			enableParallelDownloads: false,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -114,7 +114,7 @@ func TestCacheFileForRangeReadTrueTest(t *testing.T) {
 			cacheFileForRangeRead:   true,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -123,7 +123,7 @@ func TestCacheFileForRangeReadTrueTest(t *testing.T) {
 			cacheFileForRangeRead:   true,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            ramCacheDir,
 		},
 		{
@@ -132,7 +132,7 @@ func TestCacheFileForRangeReadTrueTest(t *testing.T) {
 			cacheFileForRangeRead:   true,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          false,
+			enableODirect:           true,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -141,7 +141,7 @@ func TestCacheFileForRangeReadTrueTest(t *testing.T) {
 			cacheFileForRangeRead:   true,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          false,
+			enableODirect:           true,
 			cacheDirPath:            ramCacheDir,
 		},
 		{
@@ -150,7 +150,7 @@ func TestCacheFileForRangeReadTrueTest(t *testing.T) {
 			cacheFileForRangeRead:   true,
 			fileName:                configFileName,
 			enableParallelDownloads: false,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            ramCacheDir,
 		},
 	}

tools/integration_tests/read_cache/disabled_cache_ttl_test.go

@@ -101,7 +101,7 @@ func TestDisabledCacheTTLTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileName,
 			enableParallelDownloads: false,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 		{
@@ -110,7 +110,7 @@ func TestDisabledCacheTTLTest(t *testing.T) {
 			cacheFileForRangeRead:   false,
 			fileName:                configFileNameForParallelDownloadTests,
 			enableParallelDownloads: true,
-			disableODirect:          true,
+			enableODirect:           false,
 			cacheDirPath:            getDefaultCacheDirPathForTests(),
 		},
 	}

tools/integration_tests/read_cache/job_chunk_test.go

@@ -178,7 +178,7 @@ func TestJobChunkTest(t *testing.T) {
 
 	// Tests to validate chunk size when read cache parallel downloads are disabled.
 	var chunkSizeForReadCache int64 = 8
-	ts.flags = []string{"--config-file=" + createConfigFile(&gcsfuseTestFlags{cacheSize: cacheSizeMB, cacheFileForRangeRead: true, fileName: configFileName, enableParallelDownloads: false, disableODirect: true, cacheDirPath: getDefaultCacheDirPathForTests()})}
+	ts.flags = []string{"--config-file=" + createConfigFile(&gcsfuseTestFlags{cacheSize: cacheSizeMB, cacheFileForRangeRead: true, fileName: configFileName, enableParallelDownloads: false, enableODirect: false, cacheDirPath: getDefaultCacheDirPathForTests()})}
 	ts.chunkSize = chunkSizeForReadCache * util.MiB
 	log.Printf("Running tests with flags: %s", ts.flags)
 	test_setup.RunTests(t, ts)