diff --git a/flags.go b/flags.go index e369dbb4cc..bf6d66d24c 100644 --- a/flags.go +++ b/flags.go @@ -22,6 +22,7 @@ import ( "strings" "time" + "github.com/googlecloudplatform/gcsfuse/internal/config" "github.com/googlecloudplatform/gcsfuse/internal/logger" "github.com/googlecloudplatform/gcsfuse/internal/mount" mountpkg "github.com/googlecloudplatform/gcsfuse/internal/mount" @@ -411,16 +412,24 @@ type flagStorage struct { DebugMutex bool } +func resolveFilePath(filePath string, configKey string) (resolvedPath string, err error) { + resolvedPath, err = util.GetResolvedPath(filePath) + if filePath == resolvedPath || err != nil { + return + } + + logger.Infof("Value of [%s] resolved from [%s] to [%s]\n", configKey, filePath, resolvedPath) + return resolvedPath, nil +} + // This method resolves path in the context dictionary. func resolvePathForTheFlagInContext(flagKey string, c *cli.Context) (err error) { flagValue := c.String(flagKey) - resolvedPath, err := util.ResolveFilePath(flagValue, flagKey) + resolvedPath, err := resolveFilePath(flagValue, flagKey) if err != nil { return } - logger.Infof("Value of [%s] resolved from [%s] to [%s]\n", flagKey, flagValue, resolvedPath) - err = c.Set(flagKey, resolvedPath) return } @@ -448,6 +457,23 @@ func resolvePathForTheFlagsInContext(c *cli.Context) (err error) { return } +// resolveConfigFilePaths resolves the config file paths specified in the config file. +func resolveConfigFilePaths(mountConfig *config.MountConfig) (err error) { + mountConfig.LogConfig.FilePath, err = resolveFilePath(mountConfig.LogConfig.FilePath, "logging: file") + if err != nil { + return + } + + // Resolve cache-dir path + resolvedPath, err := resolveFilePath(string(mountConfig.CacheDir), "cache-dir") + mountConfig.CacheDir = config.CacheDir(resolvedPath) + if err != nil { + return + } + + return +} + // Add the flags accepted by run to the supplied flag set, returning the // variables into which the flags will parse. func populateFlags(c *cli.Context) (flags *flagStorage, err error) { diff --git a/flags_test.go b/flags_test.go index 733706167d..d2e9a3a903 100644 --- a/flags_test.go +++ b/flags_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/googlecloudplatform/gcsfuse/internal/config" "github.com/googlecloudplatform/gcsfuse/internal/mount" mountpkg "github.com/googlecloudplatform/gcsfuse/internal/mount" . "github.com/jacobsa/oglematchers" @@ -360,3 +361,29 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientPro AssertEq(nil, err) } + +func (t *FlagsTest) Test_resolveConfigFilePaths() { + mountConfig := &config.MountConfig{} + mountConfig.LogConfig = config.LogConfig{ + FilePath: "~/test.txt", + } + mountConfig.CacheDir = "~/cache-dir" + + err := resolveConfigFilePaths(mountConfig) + + AssertEq(nil, err) + homeDir, err := os.UserHomeDir() + AssertEq(nil, err) + ExpectEq(filepath.Join(homeDir, "test.txt"), mountConfig.LogConfig.FilePath) + ExpectEq(filepath.Join(homeDir, "cache-dir"), mountConfig.CacheDir) +} + +func (t *FlagsTest) Test_resolveConfigFilePaths_WithoutSettingPaths() { + mountConfig := &config.MountConfig{} + + err := resolveConfigFilePaths(mountConfig) + + AssertEq(nil, err) + ExpectEq("", mountConfig.LogConfig.FilePath) + ExpectEq("", mountConfig.CacheDir) +} diff --git a/internal/config/config_util.go b/internal/config/config_util.go index 5bfc6d2b89..5847a7716f 100644 --- a/internal/config/config_util.go +++ b/internal/config/config_util.go @@ -14,10 +14,6 @@ package config -import ( - "github.com/googlecloudplatform/gcsfuse/internal/util" -) - // OverrideWithLoggingFlags overwrites the configs with the flag values if the // config values are empty. func OverrideWithLoggingFlags(mountConfig *MountConfig, logFile string, logFormat string, @@ -36,13 +32,3 @@ func OverrideWithLoggingFlags(mountConfig *MountConfig, logFile string, logForma mountConfig.LogConfig.Severity = TRACE } } - -// ResolveConfigFilePaths resolves the config file paths specified in the config file. -func ResolveConfigFilePaths(config *MountConfig) (err error) { - config.LogConfig.FilePath, err = util.ResolveFilePath(config.LogConfig.FilePath, "logging: file") - if err != nil { - return - } - - return -} diff --git a/internal/config/config_util_test.go b/internal/config/config_util_test.go index 1c29a98997..a420e7404f 100644 --- a/internal/config/config_util_test.go +++ b/internal/config/config_util_test.go @@ -15,8 +15,6 @@ package config import ( - "os" - "path/filepath" "testing" . "github.com/jacobsa/ogletest" @@ -90,17 +88,3 @@ func (t *ConfigTest) TestOverrideLoggingFlags_WithEmptyLogConfigs() { AssertEq(mountConfig.LogConfig.FilePath, "a.txt") AssertEq(mountConfig.LogConfig.Severity, INFO) } - -func (t *ConfigTest) TestResolveConfigFilePaths() { - mountConfig := &MountConfig{} - mountConfig.LogConfig = LogConfig{ - FilePath: "~/test.txt", - } - - err := ResolveConfigFilePaths(mountConfig) - - AssertEq(nil, err) - homeDir, err := os.UserHomeDir() - AssertEq(nil, err) - ExpectEq(filepath.Join(homeDir, "test.txt"), mountConfig.LogConfig.FilePath) -} diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 9a8d085991..54ed06f706 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -50,6 +50,8 @@ const ( // is fixed 80 bytes + length of entry-key string // (which is assumed to be 20 for common cases). AverageTypeCacheEntrySize int = 200 + + DefaultFileCacheMaxSizeMB int64 = -1 ) type WriteConfig struct { @@ -130,7 +132,7 @@ func NewMountConfig() *MountConfig { LogRotateConfig: DefaultLogRotateConfig(), } mountConfig.FileCacheConfig = FileCacheConfig{ - MaxSizeMB: 0, + MaxSizeMB: DefaultFileCacheMaxSizeMB, } mountConfig.MetadataCacheConfig = MetadataCacheConfig{ TtlInSeconds: TtlInSecsUnsetSentinel, diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index a210380eb7..db13871741 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -40,7 +40,7 @@ func validateDefaultConfig(mountConfig *MountConfig) { ExpectEq(10, mountConfig.LogConfig.LogRotateConfig.BackupFileCount) ExpectEq(true, mountConfig.LogConfig.LogRotateConfig.Compress) ExpectEq("", mountConfig.CacheDir) - ExpectEq(0, mountConfig.FileCacheConfig.MaxSizeMB) + ExpectEq(-1, mountConfig.FileCacheConfig.MaxSizeMB) ExpectEq(false, mountConfig.FileCacheConfig.CacheFileForRangeRead) } diff --git a/internal/fs/fs.go b/internal/fs/fs.go index d7ef782eb8..8258906603 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -22,7 +22,6 @@ import ( "math" "os" "path" - "path/filepath" "reflect" "strings" "syscall" @@ -154,9 +153,10 @@ func NewFileSystem( } } - // Create file cache handler if cache is enabled by user. + // Create file cache handler if cache is enabled by user. Cache is considered + // enabled only if cache-dir is not empty and file-cache:max-size-mb is non 0. var fileCacheHandler *file.CacheHandler - if cfg.MountConfig.FileCacheConfig.MaxSizeMB != 0 { + if cfg.MountConfig.FileCacheConfig.MaxSizeMB != 0 && string(cfg.MountConfig.CacheDir) != "" { fileCacheHandler = createFileCacheHandler(cfg) } @@ -224,21 +224,9 @@ func createFileCacheHandler(cfg *ServerConfig) (fileCacheHandler *file.CacheHand fileInfoCache := lru.NewCache(sizeInBytes) cacheDir := string(cfg.MountConfig.CacheDir) - // Use temp-dir as default cache-dir. - if cacheDir == "" { - if cfg.TempDir == "" { - cacheDir = os.TempDir() - } else { - cacheDir = cfg.TempDir - } - } - cacheDir, err := filepath.Abs(cacheDir) - if err != nil { - panic(fmt.Errorf("createFileCacheHandler: error while resolving cache-dir (%s) in config-file: %w", cacheDir, err)) - } - // Adding a new directory inside cacheDir, so that at the time of Destroy - // during unmount we can do os.RemoveAll(cacheDir) without deleting non - // gcsfuse related files. + // Adding a new directory inside cacheDir to keep file-cache separate from + // metadata cache if and when we support storing metadata cache on disk in + // the future. cacheDir = path.Join(cacheDir, util.FileCache) filePerm := util.DefaultFilePerm diff --git a/internal/fs/read_cache_test.go b/internal/fs/read_cache_test.go index 9e386aa8af..c04aec1a68 100644 --- a/internal/fs/read_cache_test.go +++ b/internal/fs/read_cache_test.go @@ -52,8 +52,7 @@ func init() { RegisterTestSuite(&FileCacheWithCacheForRangeRead{}) RegisterTestSuite(&FileCacheTest{}) RegisterTestSuite(&FileCacheDestroyTest{}) - RegisterTestSuite(&FileCacheWithDefaultCacheDir{}) - RegisterTestSuite(&FileCacheWithUserDefinedTempAsCacheDir{}) + RegisterTestSuite(&FileCacheIsDisabledWithCacheDirAndZeroMaxSize{}) } var CacheDir = path.Join(os.Getenv("HOME"), "cache-dir") @@ -725,57 +724,49 @@ func (t *FileCacheTest) ModifyFileInCacheAndThenReadShouldGiveModifiedData() { AssertTrue(reflect.DeepEqual(changedContent, string(buf))) } -// A collection of tests for a file system where the file cache is enabled -// with default cache location. -type FileCacheWithDefaultCacheDir struct { +// Tests for file system where the file cache is disabled if cache-dir is passed +// but file-cache: max-size-mb is 0. +type FileCacheIsDisabledWithCacheDirAndZeroMaxSize struct { fsTest } -type FileCacheWithUserDefinedTempAsCacheDir struct { - fsTest -} - -func (t *FileCacheWithDefaultCacheDir) SetUpTestSuite() { - t.serverCfg.ImplicitDirectories = true - t.serverCfg.MountConfig = &config.MountConfig{ - FileCacheConfig: config.FileCacheConfig{ - MaxSizeMB: -1, - CacheFileForRangeRead: true, - }, - } - t.fsTest.SetUpTestSuite() -} - -func (t *FileCacheWithUserDefinedTempAsCacheDir) SetUpTestSuite() { +func (t *FileCacheIsDisabledWithCacheDirAndZeroMaxSize) SetUpTestSuite() { t.serverCfg.ImplicitDirectories = true t.serverCfg.MountConfig = &config.MountConfig{ FileCacheConfig: config.FileCacheConfig{ - MaxSizeMB: -1, + MaxSizeMB: 0, CacheFileForRangeRead: true, }, + CacheDir: config.CacheDir(CacheDir), } - t.serverCfg.TempDir = UserTempLocation t.fsTest.SetUpTestSuite() } -func (t *FileCacheWithDefaultCacheDir) TearDown() { +func (t *FileCacheIsDisabledWithCacheDirAndZeroMaxSize) TearDown() { t.fsTest.TearDown() - err := os.RemoveAll(path.Join(os.TempDir(), util.FileCache)) - AssertEq(nil, err) } -func (t *FileCacheWithUserDefinedTempAsCacheDir) TearDown() { - t.fsTest.TearDown() - err := os.RemoveAll(path.Join(UserTempLocation, util.FileCache)) +func (t *FileCacheIsDisabledWithCacheDirAndZeroMaxSize) ReadingFileDoesNotPopulateCache() { + objectContent := generateRandomString(DefaultObjectSizeInMb * util.MiB) + objects := map[string]string{DefaultObjectName: objectContent} + err := t.createObjects(objects) + AssertEq(nil, err) + filePath := path.Join(mntDir, DefaultObjectName) + file, err := os.OpenFile(filePath, os.O_RDWR|syscall.O_DIRECT, util.DefaultFilePerm) + defer closeFile(file) AssertEq(nil, err) -} -func (t *FileCacheWithDefaultCacheDir) DefaultLocationIsTempDir() { - sequentialReadShouldPopulateCache(&t.fsTest, path.Join(os.TempDir(), util.FileCache)) -} + // Reading object with cache disabled should not cache the object into file. + buf := make([]byte, len(objectContent)) + _, err = file.Read(buf) + AssertEq(nil, err) + AssertEq(objectContent, string(buf)) -func (t *FileCacheWithUserDefinedTempAsCacheDir) CacheDirIsUserDefinedTempDir() { - sequentialReadShouldPopulateCache(&t.fsTest, path.Join(UserTempLocation, util.FileCache)) + objectPath := util.GetObjectPath(bucket.Name(), DefaultObjectName) + downloadPath := util.GetDownloadPath(FileCacheDir, objectPath) + _, err = os.Stat(downloadPath) + AssertNe(nil, err) + AssertTrue(os.IsNotExist(err)) } // Test to check cache is not deleted at the time of unmounting. diff --git a/internal/util/util.go b/internal/util/util.go index 00434dbb0b..88ec6d97fd 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -68,15 +68,6 @@ func GetResolvedPath(filePath string) (resolvedPath string, err error) { } } -func ResolveFilePath(filePath string, configKey string) (resolvedPath string, err error) { - resolvedPath, err = GetResolvedPath(filePath) - if filePath == resolvedPath || err != nil { - return - } - - return resolvedPath, nil -} - // Stringify marshals an object (only exported attribute) to a JSON string. If marshalling fails, it returns an empty string. func Stringify(input any) (string, error) { inputBytes, err := json.Marshal(input) diff --git a/main.go b/main.go index 8816a5ee30..5a1e73dd59 100644 --- a/main.go +++ b/main.go @@ -205,7 +205,7 @@ func runCLIApp(c *cli.Context) (err error) { config.OverrideWithLoggingFlags(mountConfig, flags.LogFile, flags.LogFormat, flags.DebugFuse, flags.DebugGCS, flags.DebugMutex) - err = config.ResolveConfigFilePaths(mountConfig) + err = resolveConfigFilePaths(mountConfig) if err != nil { return fmt.Errorf("Resolving path: %w", err) } diff --git a/perfmetrics/scripts/continuous_test/gcp_ubuntu/periodic_experiments/experiments_configuration.json b/perfmetrics/scripts/continuous_test/gcp_ubuntu/periodic_experiments/experiments_configuration.json index c181213e3f..3842bc7595 100644 --- a/perfmetrics/scripts/continuous_test/gcp_ubuntu/periodic_experiments/experiments_configuration.json +++ b/perfmetrics/scripts/continuous_test/gcp_ubuntu/periodic_experiments/experiments_configuration.json @@ -13,7 +13,8 @@ "file-cache": { "max-size-mb": -1, "cache-file-for-range-read": true - } + }, + "cache-dir": "/tmp/cache" }, "branch": "read_cache_release", "end_date": "2024-03-31 05:30:00+00:00" @@ -25,7 +26,8 @@ "file-cache": { "max-size-mb": -1, "cache-file-for-range-read": false - } + }, + "cache-dir": "/tmp/cache" }, "branch": "read_cache_release", "end_date": "2024-03-31 05:30:00+00:00"