From b5b6378e1922061c6347d589c69e5b114a4f4cfd Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 16 May 2024 15:27:57 +0000 Subject: [PATCH 01/36] Call ls -R from within mount This call runs `ls -R` on mount-directory, right after linux mount, and blocks returning after mount completion to the user until the `ls -R` completes. The `ls -R` is simulated with filepath.WalkDir which recursively visits each sub-directory and takes count of all files/directories. --- main.go | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index a3bd1a642c..e8e458f053 100644 --- a/main.go +++ b/main.go @@ -21,10 +21,12 @@ package main import ( "fmt" + "io/fs" "log" "os" "os/signal" "path" + "path/filepath" "strings" "github.com/googlecloudplatform/gcsfuse/v2/cmd" @@ -208,6 +210,24 @@ func populateArgs(c *cli.Context) ( return } +func callListRecursive(mountPoint string) (err error) { + logger.Debugf("Started recursive listing of %s ...", mountPoint) + numItems := 0 + err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { + //logger.Infof("Walked path:%s, d=%s, isDir=%v", path, d.Name(), d.IsDir()) + if err != nil { + logger.Errorf("Walked path:%s, d=%s, isDir=%v, err=%v", path, d.Name(), d.IsDir(), err) + } + + numItems++ + return err + }) + + logger.Debugf("... Completed recursive listing of %s. Number of items listed: %v", mountPoint, numItems) + + return +} + func runCLIApp(c *cli.Context) (err error) { err = resolvePathForTheFlagsInContext(c) if err != nil { @@ -366,6 +386,12 @@ func runCLIApp(c *cli.Context) (err error) { return } else { logger.Infof(SuccessfulMountMessage) + + err = callListRecursive(mountPoint) + if err != nil { + err = fmt.Errorf("daemonize.Run: %w", err) + return + } } return @@ -385,7 +411,18 @@ func runCLIApp(c *cli.Context) (err error) { // Print the success message in the log-file/stdout depending on what the logger is set to. logger.Info(SuccessfulMountMessage) - daemonize.SignalOutcome(nil) + err = callListRecursive(mountPoint) + if err != nil { + // Printing via mountStatus will have duplicate logs on the console while + // mounting gcsfuse in foreground mode. But this is important to avoid + // losing error logs when run in the background mode. + logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) + err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) + daemonize.SignalOutcome(err) + return + } else { + daemonize.SignalOutcome(nil) + } } else { // Printing via mountStatus will have duplicate logs on the console while // mounting gcsfuse in foreground mode. But this is important to avoid From c6cc8dcf7f1a9da6335a1da5ce46d87782f67a0b Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 05:53:57 +0000 Subject: [PATCH 02/36] Temp - [WIP] concurrent listings of sibling directories --- main.go | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index e8e458f053..1602c25f10 100644 --- a/main.go +++ b/main.go @@ -210,15 +210,66 @@ func populateArgs(c *cli.Context) ( return } +// walkDir recursively descends path, calling walkDirFn. +func walkDirConcurrent(path string, d fs.DirEntry, walkDirFn fs.WalkDirFunc) error { + if err := walkDirFn(path, d, nil); err != nil || !d.IsDir() { + if err == filepath.SkipDir && d.IsDir() { + // Successfully skipped directory. + err = nil + } + return err + } + + dirs, err := os.ReadDir(path) + if err != nil { + // Second call, to report ReadDir error. + err = walkDirFn(path, d, err) + if err != nil { + if err == filepath.SkipDir && d.IsDir() { + err = nil + } + return err + } + } + + for _, d1 := range dirs { + path1 := filepath.Join(path, d1.Name()) + if err := walkDirConcurrent(path1, d1, walkDirFn); err != nil { + if err == filepath.SkipDir { + break + } + return err + } + } + return nil +} + +func WalkDirConcurrent(root string, fn fs.WalkDirFunc) error { + info, err := os.Lstat(root) + if err != nil { + err = fn(root, nil, err) + } else { + err = walkDirConcurrent(root, fs.FileInfoToDirEntry(info), fn) + } + if err == filepath.SkipDir || err == filepath.SkipAll { + return nil + } + return err +} + func callListRecursive(mountPoint string) (err error) { logger.Debugf("Started recursive listing of %s ...", mountPoint) numItems := 0 - err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { + err = WalkDirConcurrent(mountPoint, func(path string, d fs.DirEntry, err error) error { //logger.Infof("Walked path:%s, d=%s, isDir=%v", path, d.Name(), d.IsDir()) if err != nil { logger.Errorf("Walked path:%s, d=%s, isDir=%v, err=%v", path, d.Name(), d.IsDir(), err) } + if d.IsDir() && path != mountPoint { + logger.Debugf("Started recursive listing of directory %s", path) + } + numItems++ return err }) From a24e655c77b6c44db058119af5601d2ebafc3a93 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 05:55:55 +0000 Subject: [PATCH 03/36] Revert "Temp - [WIP] concurrent listings of sibling directories" This reverts commit c34e77f09dfc995769ff5a004a127c2c21d0c76e. --- main.go | 53 +---------------------------------------------------- 1 file changed, 1 insertion(+), 52 deletions(-) diff --git a/main.go b/main.go index 1602c25f10..e8e458f053 100644 --- a/main.go +++ b/main.go @@ -210,66 +210,15 @@ func populateArgs(c *cli.Context) ( return } -// walkDir recursively descends path, calling walkDirFn. -func walkDirConcurrent(path string, d fs.DirEntry, walkDirFn fs.WalkDirFunc) error { - if err := walkDirFn(path, d, nil); err != nil || !d.IsDir() { - if err == filepath.SkipDir && d.IsDir() { - // Successfully skipped directory. - err = nil - } - return err - } - - dirs, err := os.ReadDir(path) - if err != nil { - // Second call, to report ReadDir error. - err = walkDirFn(path, d, err) - if err != nil { - if err == filepath.SkipDir && d.IsDir() { - err = nil - } - return err - } - } - - for _, d1 := range dirs { - path1 := filepath.Join(path, d1.Name()) - if err := walkDirConcurrent(path1, d1, walkDirFn); err != nil { - if err == filepath.SkipDir { - break - } - return err - } - } - return nil -} - -func WalkDirConcurrent(root string, fn fs.WalkDirFunc) error { - info, err := os.Lstat(root) - if err != nil { - err = fn(root, nil, err) - } else { - err = walkDirConcurrent(root, fs.FileInfoToDirEntry(info), fn) - } - if err == filepath.SkipDir || err == filepath.SkipAll { - return nil - } - return err -} - func callListRecursive(mountPoint string) (err error) { logger.Debugf("Started recursive listing of %s ...", mountPoint) numItems := 0 - err = WalkDirConcurrent(mountPoint, func(path string, d fs.DirEntry, err error) error { + err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { //logger.Infof("Walked path:%s, d=%s, isDir=%v", path, d.Name(), d.IsDir()) if err != nil { logger.Errorf("Walked path:%s, d=%s, isDir=%v, err=%v", path, d.Name(), d.IsDir(), err) } - if d.IsDir() && path != mountPoint { - logger.Debugf("Started recursive listing of directory %s", path) - } - numItems++ return err }) From 7c356e260ff3358ade4c324dd5014adb9973f265 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 08:57:03 +0000 Subject: [PATCH 04/36] Add mount-config metadata-prefetch-mode Added mountconfig-flag "metadata-prefetch-mode' for config-file. The type of value passed is a string. The supported values are: (a) disabled, (b) synchronous, (c) asynchronous. If not set, the value `disabled` is taken by default. If value `synchronous` is set, then mount is blocked until the metadata prefetch is complete. In this mode, if the metadata-prefetch fails, the mount itself fails. If value is set to `asynchronous`, then mount is not blocked, but metadata prefetch is initiated in a go-routine and will complete sometime later. In this case, if the metadata-prefetch fails, then an error is logged in the GCSFuse log-file set using log-file in commandline flags, or file in `log-config: file` in config-file. Metadata-prefetch is equivalent to running `ls -R` on the mount-directory to prime/prefill/prefetch the metadata caches (both type and stat). --- internal/config/mount_config.go | 17 +++++ ...tadata_prefetch_mode_supported_value1.yaml | 1 + ...tadata_prefetch_mode_supported_value2.yaml | 1 + ...tadata_prefetch_mode_supported_value3.yaml | 1 + .../metadata_prefetch_mode_unset.yaml | 0 ...adata_prefetch_mode_unsupported_value.yaml | 1 + internal/config/yaml_parser.go | 19 +++++ internal/config/yaml_parser_test.go | 43 +++++++++++ main.go | 76 +++++++++++++------ main_test.go | 29 ++++++- 10 files changed, 163 insertions(+), 25 deletions(-) create mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml create mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml create mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml create mode 100644 internal/config/testdata/metadata_prefetch_mode_unset.yaml create mode 100644 internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 715a6a100d..5a67d14401 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -19,6 +19,8 @@ import ( "time" ) +type MetadataPrefetchModeValue string + const ( // Default log rotation config values. defaultMaxFileSizeMB = 512 @@ -50,6 +52,11 @@ const ( DefaultGrpcConnPoolSize = 1 DefaultAnonymousAccess = false DefaultEnableHNS = false + + MetadataPrefetchModeDisabled MetadataPrefetchModeValue = "disabled" + MetadataPrefetchModeSynchronous MetadataPrefetchModeValue = "synchronous" + MetadataPrefetchModeAsynchronous MetadataPrefetchModeValue = "asynchronous" + DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled ) type WriteConfig struct { @@ -130,6 +137,15 @@ type MountConfig struct { AuthConfig `yaml:"auth-config"` EnableHNS `yaml:"enable-hns"` FileSystemConfig `yaml:"file-system"` + + // MetadataPrefetchMode indicates whether or not to prefetch the metadata of the mounted bucket. + // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. + // Supported values: + // "disabled": no pretch is executed. This is the default value, if not set. + // "synchronous": prefetch is executed with, blocking mount. + // "asynchronous": prefetch is executed without blocking mount. + // Any other values will return error on mounting. + MetadataPrefetchMode MetadataPrefetchModeValue `yaml:"metadata-prefetch-mode"` } // LogRotateConfig defines the parameters for log rotation. It consists of three @@ -181,5 +197,6 @@ func NewMountConfig() *MountConfig { AnonymousAccess: DefaultAnonymousAccess, } mountConfig.EnableHNS = DefaultEnableHNS + mountConfig.MetadataPrefetchMode = DefaultMetadataPrefetchMode return mountConfig } diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml new file mode 100644 index 0000000000..623112bcb5 --- /dev/null +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml @@ -0,0 +1 @@ +metadata-prefetch-mode: synchronous \ No newline at end of file diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml new file mode 100644 index 0000000000..8fb95287f5 --- /dev/null +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml @@ -0,0 +1 @@ +metadata-prefetch-mode: asynchronous \ No newline at end of file diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml new file mode 100644 index 0000000000..d13dc30030 --- /dev/null +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml @@ -0,0 +1 @@ +metadata-prefetch-mode: disabled \ No newline at end of file diff --git a/internal/config/testdata/metadata_prefetch_mode_unset.yaml b/internal/config/testdata/metadata_prefetch_mode_unset.yaml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml b/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml new file mode 100644 index 0000000000..10c6496837 --- /dev/null +++ b/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml @@ -0,0 +1 @@ +metadata-prefetch-mode: unsupported \ No newline at end of file diff --git a/internal/config/yaml_parser.go b/internal/config/yaml_parser.go index 6cbab05e29..605a247465 100644 --- a/internal/config/yaml_parser.go +++ b/internal/config/yaml_parser.go @@ -43,6 +43,7 @@ const ( StatCacheMaxSizeMBInvalidValueError = "the value of stat-cache-max-size-mb for metadata-cache can't be less than -1" StatCacheMaxSizeMBTooHighError = "the value of stat-cache-max-size-mb for metadata-cache is too high! Max supported: 17592186044415" MaxSupportedStatCacheMaxSizeMB = util.MaxMiBsInUint64 + UnsupportedMetadataPrefixModeError = "unsupported metadata-prefix-mode: \"%s\"; supported values: disabled, synchronous, asynchronous." ) func IsValidLogSeverity(severity LogSeverity) bool { @@ -107,6 +108,19 @@ func (grpcClientConfig *GrpcClientConfig) validate() error { return nil } +func validateMetadataPrefetchMode(mode MetadataPrefetchModeValue) error { + switch mode { + case MetadataPrefetchModeDisabled: + fallthrough + case MetadataPrefetchModeSynchronous: + fallthrough + case MetadataPrefetchModeAsynchronous: + return nil + default: + return fmt.Errorf(UnsupportedMetadataPrefixModeError, mode) + } +} + func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) { mountConfig = NewMountConfig() @@ -155,5 +169,10 @@ func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) { if err = mountConfig.GrpcClientConfig.validate(); err != nil { return mountConfig, fmt.Errorf("error parsing grpc-config: %w", err) } + + if err = validateMetadataPrefetchMode(mountConfig.MetadataPrefetchMode); err != nil { + return mountConfig, fmt.Errorf("error parsing metadata-prefetch-mode: %w", err) + } + return } diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index ac667140d2..f21ade3c1e 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -128,6 +128,9 @@ func (t *YamlParserTest) TestReadConfigFile_ValidConfig() { // file-system config assert.True(t.T(), mountConfig.FileSystemConfig.IgnoreInterrupts) assert.True(t.T(), mountConfig.FileSystemConfig.DisableParallelDirops) + + // metadata-prefetch-mode config + assert.Equal(t.T(), "disabled", string(mountConfig.MetadataPrefetchMode)) } func (t *YamlParserTest) TestReadConfigFile_InvalidLogConfig() { @@ -263,3 +266,43 @@ func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetDisableParalle assert.NotNil(t.T(), mountConfig) assert.False(t.T(), mountConfig.FileSystemConfig.DisableParallelDirops) } + +func (t *YamlParserTest) TestReadConfigFile_MetatadaPrefetchMode_SupportedValues() { + for _, input := range []struct { + filename string + mode string + }{{ + filename: "testdata/metadata_prefetch_mode_supported_value1.yaml", + mode: "synchronous", + }, + { + filename: "testdata/metadata_prefetch_mode_supported_value2.yaml", + mode: "asynchronous", + }, + { + filename: "testdata/metadata_prefetch_mode_supported_value3.yaml", + mode: "disabled", + }, + { + filename: "testdata/metadata_prefetch_mode_unset.yaml", + mode: "disabled", + }, + } { + mountConfig, err := ParseConfigFile(input.filename) + assert.NoError(t.T(), err) + assert.NotNil(t.T(), mountConfig) + assert.Equal(t.T(), input.mode, string(mountConfig.MetadataPrefetchMode)) + } +} + +func (t *YamlParserTest) TestReadConfigFile_MetatadaPrefetchMode_UnsupportedValues() { + for _, input := range []struct { + filename string + }{{ + filename: "testdata/metadata_prefetch_mode_unsupported_value.yaml", + }, + } { + _, err := ParseConfigFile(input.filename) + assert.ErrorContains(t.T(), err, fmt.Sprintf(UnsupportedMetadataPrefixModeError, "unsupported")) + } +} diff --git a/main.go b/main.go index e8e458f053..7149818760 100644 --- a/main.go +++ b/main.go @@ -211,19 +211,19 @@ func populateArgs(c *cli.Context) ( } func callListRecursive(mountPoint string) (err error) { - logger.Debugf("Started recursive listing of %s ...", mountPoint) + logger.Debugf("Started recursive metadata-prefetch of directory: \"%s\" ...", mountPoint) numItems := 0 err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { //logger.Infof("Walked path:%s, d=%s, isDir=%v", path, d.Name(), d.IsDir()) if err != nil { - logger.Errorf("Walked path:%s, d=%s, isDir=%v, err=%v", path, d.Name(), d.IsDir(), err) + return fmt.Errorf("got error walking: path=\"%s\", dentry=\"%s\", isDir=%v, err=%w", path, d.Name(), d.IsDir(), err) } numItems++ return err }) - logger.Debugf("... Completed recursive listing of %s. Number of items listed: %v", mountPoint, numItems) + logger.Debugf("... Completed recursive metadata-prefetch of directory: \"%s\". Number of items discovered: %v", mountPoint, numItems) return } @@ -385,12 +385,23 @@ func runCLIApp(c *cli.Context) (err error) { err = fmt.Errorf("daemonize.Run: %w", err) return } else { - logger.Infof(SuccessfulMountMessage) - - err = callListRecursive(mountPoint) - if err != nil { - err = fmt.Errorf("daemonize.Run: %w", err) - return + switch mountConfig.MetadataPrefetchMode { + case config.MetadataPrefetchModeDisabled: + logger.Infof(SuccessfulMountMessage) + case config.MetadataPrefetchModeSynchronous: + err = callListRecursive(mountPoint) + if err != nil { + err = fmt.Errorf("daemonize.Run: metadata-prefetch failed: %w", err) + return + } + logger.Infof(SuccessfulMountMessage) + case config.MetadataPrefetchModeAsynchronous: + logger.Infof(SuccessfulMountMessage) + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("metadata-prefetch failed: %v", err) + } + }() } } @@ -407,21 +418,42 @@ func runCLIApp(c *cli.Context) (err error) { { mfs, err = mountWithArgs(bucketName, mountPoint, flags, mountConfig) - if err == nil { - // Print the success message in the log-file/stdout depending on what the logger is set to. - logger.Info(SuccessfulMountMessage) + callDaemonizeSignalOutcome := func(err error) { + if err2 := daemonize.SignalOutcome(err); err2 != nil { + logger.Errorf("Failed to signal to daemon: %v", err2) + } + } - err = callListRecursive(mountPoint) - if err != nil { - // Printing via mountStatus will have duplicate logs on the console while - // mounting gcsfuse in foreground mode. But this is important to avoid - // losing error logs when run in the background mode. - logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) - err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) - daemonize.SignalOutcome(err) - return - } else { + if err == nil { + switch mountConfig.MetadataPrefetchMode { + case config.MetadataPrefetchModeDisabled: + // Print the success message in the log-file/stdout depending on what the logger is set to. + logger.Info(SuccessfulMountMessage) + callDaemonizeSignalOutcome(nil) + case config.MetadataPrefetchModeSynchronous: + if err = callListRecursive(mountPoint); err != nil { + // Printing via mountStatus will have duplicate logs on the console while + // mounting gcsfuse in foreground mode. But this is important to avoid + // losing error logs when run in the background mode. + logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) + err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) + callDaemonizeSignalOutcome(err) + return + } + + // Print the success message in the log-file/stdout depending on what the logger is set to. + logger.Info(SuccessfulMountMessage) daemonize.SignalOutcome(nil) + case config.MetadataPrefetchModeAsynchronous: + // Print the success message in the log-file/stdout depending on what the logger is set to. + logger.Info(SuccessfulMountMessage) + callDaemonizeSignalOutcome(nil) + + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("Metadata-prefetch failed: %v", err) + } + }() } } else { // Printing via mountStatus will have duplicate logs on the console while diff --git a/main_test.go b/main_test.go index 5af325b7f1..3fb0a6bbfe 100644 --- a/main_test.go +++ b/main_test.go @@ -176,7 +176,7 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInMountConfigAsMarshal actual, err := util.Stringify(mountConfig) AssertEq(nil, err) - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"TRACE\",\"Format\":\"\",\"FilePath\":\"\\\"path\\\"to\\\"file\\\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":2,\"BackupFileCount\":2,\"Compress\":true},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":true,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false}" + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"TRACE\",\"Format\":\"\",\"FilePath\":\"\\\"path\\\"to\\\"file\\\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":2,\"BackupFileCount\":2,\"Compress\":true},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":true,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"\"}" AssertEq(expected, actual) } @@ -188,7 +188,31 @@ func (t *MainTest) TestEnableHNSFlagFalse() { actual, err := util.Stringify(mountConfig) AssertEq(nil, err) - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false}" + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"\"}" + AssertEq(expected, actual) +} + +func (t *MainTest) TestMetadataPrefetchModeSynchronous() { + mountConfig := &config.MountConfig{ + MetadataPrefetchMode: config.MetadataPrefetchModeSynchronous, + } + + actual, err := util.Stringify(mountConfig) + AssertEq(nil, err) + + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"synchronous\"}" + AssertEq(expected, actual) +} + +func (t *MainTest) TestMetadataPrefetchModeAsynchronous() { + mountConfig := &config.MountConfig{ + MetadataPrefetchMode: config.MetadataPrefetchModeAsynchronous, + } + + actual, err := util.Stringify(mountConfig) + AssertEq(nil, err) + + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"asynchronous\"}" AssertEq(expected, actual) } @@ -209,5 +233,4 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false}" AssertEq(expected, actual) - AssertEq(expected, actual) } From 4e606c61a4991f289d10805216bbef712d24a272 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 10:20:26 +0000 Subject: [PATCH 05/36] minor improvements --- internal/config/mount_config.go | 8 +++++++- main.go | 26 ++++++++++++++------------ main_test.go | 12 ++++++++++++ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 5a67d14401..b705733722 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -19,6 +19,8 @@ import ( "time" ) +// Special type declared to protect just any string +// from being set to MetadataPrefetchMode. type MetadataPrefetchModeValue string const ( @@ -52,10 +54,14 @@ const ( DefaultGrpcConnPoolSize = 1 DefaultAnonymousAccess = false DefaultEnableHNS = false - + + // MetadataPrefetchModeDisabled is the mode without metadata-prefetch. MetadataPrefetchModeDisabled MetadataPrefetchModeValue = "disabled" + // MetadataPrefetchModeSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. MetadataPrefetchModeSynchronous MetadataPrefetchModeValue = "synchronous" + // MetadataPrefetchModeAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. MetadataPrefetchModeAsynchronous MetadataPrefetchModeValue = "asynchronous" + // DefaultMetadataPrefetchMode is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchModeDisabled. DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled ) diff --git a/main.go b/main.go index 7149818760..8f4b8cef5b 100644 --- a/main.go +++ b/main.go @@ -389,8 +389,7 @@ func runCLIApp(c *cli.Context) (err error) { case config.MetadataPrefetchModeDisabled: logger.Infof(SuccessfulMountMessage) case config.MetadataPrefetchModeSynchronous: - err = callListRecursive(mountPoint) - if err != nil { + if err = callListRecursive(mountPoint); err != nil { err = fmt.Errorf("daemonize.Run: metadata-prefetch failed: %w", err) return } @@ -418,18 +417,25 @@ func runCLIApp(c *cli.Context) (err error) { { mfs, err = mountWithArgs(bucketName, mountPoint, flags, mountConfig) + // This utility is to absorb the error + // returned by daemonize.SignalOutcome calls by simply + // logging them as error logs. callDaemonizeSignalOutcome := func(err error) { if err2 := daemonize.SignalOutcome(err); err2 != nil { logger.Errorf("Failed to signal to daemon: %v", err2) } } + markSuccessfullMount := func() { + // Print the success message in the log-file/stdout depending on what the logger is set to. + logger.Info(SuccessfulMountMessage) + callDaemonizeSignalOutcome(nil) + } + if err == nil { switch mountConfig.MetadataPrefetchMode { case config.MetadataPrefetchModeDisabled: - // Print the success message in the log-file/stdout depending on what the logger is set to. - logger.Info(SuccessfulMountMessage) - callDaemonizeSignalOutcome(nil) + markSuccessfullMount() case config.MetadataPrefetchModeSynchronous: if err = callListRecursive(mountPoint); err != nil { // Printing via mountStatus will have duplicate logs on the console while @@ -441,13 +447,9 @@ func runCLIApp(c *cli.Context) (err error) { return } - // Print the success message in the log-file/stdout depending on what the logger is set to. - logger.Info(SuccessfulMountMessage) - daemonize.SignalOutcome(nil) + markSuccessfullMount() case config.MetadataPrefetchModeAsynchronous: - // Print the success message in the log-file/stdout depending on what the logger is set to. - logger.Info(SuccessfulMountMessage) - callDaemonizeSignalOutcome(nil) + markSuccessfullMount() go func() { if err := callListRecursive(mountPoint); err != nil { @@ -461,7 +463,7 @@ func runCLIApp(c *cli.Context) (err error) { // losing error logs when run in the background mode. logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) - daemonize.SignalOutcome(err) + callDaemonizeSignalOutcome(err) return } } diff --git a/main_test.go b/main_test.go index 3fb0a6bbfe..1092d5fccc 100644 --- a/main_test.go +++ b/main_test.go @@ -216,6 +216,18 @@ func (t *MainTest) TestMetadataPrefetchModeAsynchronous() { AssertEq(expected, actual) } +func (t *MainTest) TestMetadataPrefetchModeDisabled() { + mountConfig := &config.MountConfig{ + MetadataPrefetchMode: config.MetadataPrefetchModeDisabled, + } + + actual, err := util.Stringify(mountConfig) + AssertEq(nil, err) + + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"disabled\"}" + AssertEq(expected, actual) +} + func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshalledString() { mountOptions := map[string]string{ "1": "one", From 5ba736b2015d62c01c0b3b01e053b5b9ee8afe9d Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 11:37:42 +0000 Subject: [PATCH 06/36] Migrate main_test.go to stretchr/testify --- main_test.go | 58 +++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/main_test.go b/main_test.go index 1092d5fccc..42a758902d 100644 --- a/main_test.go +++ b/main_test.go @@ -10,20 +10,22 @@ import ( "github.com/googlecloudplatform/gcsfuse/v2/internal/util" mountpkg "github.com/googlecloudplatform/gcsfuse/v2/internal/mount" - . "github.com/jacobsa/ogletest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" ) -func Test_Main(t *testing.T) { RunTests(t) } +func Test_Main(t *testing.T) { + suite.Run(t, new(MainTest)) +} //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// type MainTest struct { + suite.Suite } -func init() { RegisterTestSuite(&MainTest{}) } - func (t *MainTest) TestCreateStorageHandle() { flags := &flagStorage{ ClientProtocol: mountpkg.HTTP1, @@ -40,8 +42,8 @@ func (t *MainTest) TestCreateStorageHandle() { userAgent := "AppName" storageHandle, err := createStorageHandle(flags, mountConfig, userAgent) - AssertEq(nil, err) - AssertNe(nil, storageHandle) + assert.Equal(t.T(), nil, err) + assert.NotEqual(t.T(), nil, storageHandle) } func (t *MainTest) TestCreateStorageHandle_WithClientProtocolAsGRPC() { @@ -62,8 +64,8 @@ func (t *MainTest) TestCreateStorageHandle_WithClientProtocolAsGRPC() { userAgent := "AppName" storageHandle, err := createStorageHandle(flags, mountConfig, userAgent) - AssertEq(nil, err) - AssertNe(nil, storageHandle) + assert.Equal(t.T(), nil, err) + assert.NotEqual(t.T(), nil, storageHandle) } func (t *MainTest) TestGetUserAgentWhenMetadataImageTypeEnvVarIsSet() { @@ -74,7 +76,7 @@ func (t *MainTest) TestGetUserAgentWhenMetadataImageTypeEnvVarIsSet() { userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s AppName (GPN:gcsfuse-DLVM) (Cfg:0:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentWhenMetadataImageTypeEnvVarIsNotSet() { @@ -82,14 +84,14 @@ func (t *MainTest) TestGetUserAgentWhenMetadataImageTypeEnvVarIsNotSet() { userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-AppName) (Cfg:0:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentConfigWithNoFileCache() { mountConfig := &config.MountConfig{} userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-AppName) (Cfg:0:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentConfigWithFileCacheEnabledRandomReadEnabled() { @@ -102,7 +104,7 @@ func (t *MainTest) TestGetUserAgentConfigWithFileCacheEnabledRandomReadEnabled() } userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-AppName) (Cfg:1:1)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentConfigWithFileCacheEnabledRandomDisabled() { @@ -115,7 +117,7 @@ func (t *MainTest) TestGetUserAgentConfigWithFileCacheEnabledRandomDisabled() { } userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-AppName) (Cfg:1:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentConfigWithFileCacheSizeSetCacheDirNotSet() { // Test File cache disabled where MaxSize is set but Cache Dir is not set. @@ -126,7 +128,7 @@ func (t *MainTest) TestGetUserAgentConfigWithFileCacheSizeSetCacheDirNotSet() { } userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-AppName) (Cfg:0:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentConfigWithCacheDirSetMaxSizeDisabled() { @@ -139,7 +141,7 @@ func (t *MainTest) TestGetUserAgentConfigWithCacheDirSetMaxSizeDisabled() { } userAgent := getUserAgent("AppName", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-AppName) (Cfg:0:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestGetUserAgentWhenMetadataImageTypeEnvVarSetAndAppNameNotSet() { @@ -150,7 +152,7 @@ func (t *MainTest) TestGetUserAgentWhenMetadataImageTypeEnvVarSetAndAppNameNotSe userAgent := getUserAgent("", getConfigForUserAgent(mountConfig)) expectedUserAgent := strings.TrimSpace(fmt.Sprintf("gcsfuse/%s (GPN:gcsfuse-DLVM) (Cfg:0:0)", getVersion())) - ExpectEq(expectedUserAgent, userAgent) + assert.Equal(t.T(), expectedUserAgent, userAgent) } func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInMountConfigAsMarshalledString() { @@ -174,10 +176,10 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInMountConfigAsMarshal } actual, err := util.Stringify(mountConfig) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) expected := "{\"CreateEmptyFile\":false,\"Severity\":\"TRACE\",\"Format\":\"\",\"FilePath\":\"\\\"path\\\"to\\\"file\\\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":2,\"BackupFileCount\":2,\"Compress\":true},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":true,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"\"}" - AssertEq(expected, actual) + assert.Equal(t.T(), expected, actual) } func (t *MainTest) TestEnableHNSFlagFalse() { @@ -186,10 +188,10 @@ func (t *MainTest) TestEnableHNSFlagFalse() { } actual, err := util.Stringify(mountConfig) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"\"}" - AssertEq(expected, actual) + assert.Equal(t.T(), expected, actual) } func (t *MainTest) TestMetadataPrefetchModeSynchronous() { @@ -198,10 +200,10 @@ func (t *MainTest) TestMetadataPrefetchModeSynchronous() { } actual, err := util.Stringify(mountConfig) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"synchronous\"}" - AssertEq(expected, actual) + assert.Equal(t.T(), expected, actual) } func (t *MainTest) TestMetadataPrefetchModeAsynchronous() { @@ -210,10 +212,10 @@ func (t *MainTest) TestMetadataPrefetchModeAsynchronous() { } actual, err := util.Stringify(mountConfig) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"asynchronous\"}" - AssertEq(expected, actual) + assert.Equal(t.T(), expected, actual) } func (t *MainTest) TestMetadataPrefetchModeDisabled() { @@ -222,10 +224,10 @@ func (t *MainTest) TestMetadataPrefetchModeDisabled() { } actual, err := util.Stringify(mountConfig) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"disabled\"}" - AssertEq(expected, actual) + assert.Equal(t.T(), expected, actual) } func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshalledString() { @@ -241,8 +243,8 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal } actual, err := util.Stringify(flags) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false}" - AssertEq(expected, actual) + assert.Equal(t.T(), expected, actual) } From 824c3aa02ab8588673988ca0aeee6c886257b788 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 11:45:11 +0000 Subject: [PATCH 07/36] metadata-prefetch-mode configs in listing e2e tests --- .../operations/operations_test.go | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tools/integration_tests/operations/operations_test.go b/tools/integration_tests/operations/operations_test.go index 2a5bb8db75..c0b0291286 100644 --- a/tools/integration_tests/operations/operations_test.go +++ b/tools/integration_tests/operations/operations_test.go @@ -1,4 +1,4 @@ -// Copyright 2023 Google Inc. All Rights Reserved. +// Copyright 2024 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -132,6 +132,28 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { filePath3 := setup.YAMLConfigFile(mountConfig3, "config3.yaml") flags = append(flags, []string{"--config-file=" + filePath3}) + // Set up config file testing metadata-prefetch-mode: "asynchronous". + mountConfig4 := config.MountConfig{ + LogConfig: config.LogConfig{ + Severity: config.TRACE, + LogRotateConfig: config.DefaultLogRotateConfig(), + }, + MetadataPrefetchMode: config.MetadataPrefetchModeAsynchronous, + } + filePath4 := setup.YAMLConfigFile(mountConfig4, "config4.yaml") + flags = append(flags, []string{"--config-file=" + filePath4}) + + // Set up config file testing metadata-prefetch-mode: "synchronous". + mountConfig5 := config.MountConfig{ + LogConfig: config.LogConfig{ + Severity: config.TRACE, + LogRotateConfig: config.DefaultLogRotateConfig(), + }, + MetadataPrefetchMode: config.MetadataPrefetchModeSynchronous, + } + filePath5 := setup.YAMLConfigFile(mountConfig5, "config5.yaml") + flags = append(flags, []string{"--config-file=" + filePath5}) + return flags } From 419d7a07e223af31fd267fd3f9343dd3821a894a Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Mon, 20 May 2024 11:55:59 +0000 Subject: [PATCH 08/36] fix lint errors --- internal/config/mount_config.go | 10 +++++----- .../metadata_prefetch_mode_supported_value1.yaml | 2 +- .../metadata_prefetch_mode_supported_value2.yaml | 2 +- .../metadata_prefetch_mode_supported_value3.yaml | 2 +- .../metadata_prefetch_mode_unsupported_value.yaml | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index b705733722..938ddea203 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -19,7 +19,7 @@ import ( "time" ) -// Special type declared to protect just any string +// Special type declared to stop any random string // from being set to MetadataPrefetchMode. type MetadataPrefetchModeValue string @@ -54,15 +54,15 @@ const ( DefaultGrpcConnPoolSize = 1 DefaultAnonymousAccess = false DefaultEnableHNS = false - + // MetadataPrefetchModeDisabled is the mode without metadata-prefetch. - MetadataPrefetchModeDisabled MetadataPrefetchModeValue = "disabled" + MetadataPrefetchModeDisabled MetadataPrefetchModeValue = "disabled" // MetadataPrefetchModeSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. - MetadataPrefetchModeSynchronous MetadataPrefetchModeValue = "synchronous" + MetadataPrefetchModeSynchronous MetadataPrefetchModeValue = "synchronous" // MetadataPrefetchModeAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. MetadataPrefetchModeAsynchronous MetadataPrefetchModeValue = "asynchronous" // DefaultMetadataPrefetchMode is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchModeDisabled. - DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled + DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled ) type WriteConfig struct { diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml index 623112bcb5..36d892de2e 100644 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml @@ -1 +1 @@ -metadata-prefetch-mode: synchronous \ No newline at end of file +metadata-prefetch-mode: synchronous diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml index 8fb95287f5..376808e6b5 100644 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml @@ -1 +1 @@ -metadata-prefetch-mode: asynchronous \ No newline at end of file +metadata-prefetch-mode: asynchronous diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml index d13dc30030..021b566f86 100644 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml @@ -1 +1 @@ -metadata-prefetch-mode: disabled \ No newline at end of file +metadata-prefetch-mode: disabled diff --git a/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml b/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml index 10c6496837..0e2b7b7b89 100644 --- a/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml +++ b/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml @@ -1 +1 @@ -metadata-prefetch-mode: unsupported \ No newline at end of file +metadata-prefetch-mode: unsupported From f434cd4484270d1d870ac607b75ab5ea55847d66 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 04:36:54 +0000 Subject: [PATCH 09/36] temp-fix for integration-test failure --- internal/config/yaml_parser.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/config/yaml_parser.go b/internal/config/yaml_parser.go index 605a247465..49408303d6 100644 --- a/internal/config/yaml_parser.go +++ b/internal/config/yaml_parser.go @@ -43,7 +43,7 @@ const ( StatCacheMaxSizeMBInvalidValueError = "the value of stat-cache-max-size-mb for metadata-cache can't be less than -1" StatCacheMaxSizeMBTooHighError = "the value of stat-cache-max-size-mb for metadata-cache is too high! Max supported: 17592186044415" MaxSupportedStatCacheMaxSizeMB = util.MaxMiBsInUint64 - UnsupportedMetadataPrefixModeError = "unsupported metadata-prefix-mode: \"%s\"; supported values: disabled, synchronous, asynchronous." + UnsupportedMetadataPrefixModeError = "unsupported metadata-prefix-mode: \"%s\"; supported values: disabled, synchronous, asynchronous" ) func IsValidLogSeverity(severity LogSeverity) bool { @@ -114,6 +114,9 @@ func validateMetadataPrefetchMode(mode MetadataPrefetchModeValue) error { fallthrough case MetadataPrefetchModeSynchronous: fallthrough + case MetadataPrefetchModeValue(""): + // this case is a temp-fix to avoid errors like: error parsing metadata-prefetch-mode: unsupported metadata-prefix-mode: \"\"; + fallthrough case MetadataPrefetchModeAsynchronous: return nil default: From 7e57d36c614f3de22099443bb06bcdf2f8d2f10f Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 07:33:00 +0000 Subject: [PATCH 10/36] fix some self-comments --- internal/config/mount_config.go | 20 +++++++++----------- internal/config/yaml_parser.go | 10 ++++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 938ddea203..2a99ae7edb 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -21,7 +21,7 @@ import ( // Special type declared to stop any random string // from being set to MetadataPrefetchMode. -type MetadataPrefetchModeValue string +type metadataPrefetchModeValue string const ( // Default log rotation config values. @@ -56,11 +56,11 @@ const ( DefaultEnableHNS = false // MetadataPrefetchModeDisabled is the mode without metadata-prefetch. - MetadataPrefetchModeDisabled MetadataPrefetchModeValue = "disabled" + MetadataPrefetchModeDisabled metadataPrefetchModeValue = "disabled" // MetadataPrefetchModeSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. - MetadataPrefetchModeSynchronous MetadataPrefetchModeValue = "synchronous" + MetadataPrefetchModeSynchronous metadataPrefetchModeValue = "synchronous" // MetadataPrefetchModeAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. - MetadataPrefetchModeAsynchronous MetadataPrefetchModeValue = "asynchronous" + MetadataPrefetchModeAsynchronous metadataPrefetchModeValue = "asynchronous" // DefaultMetadataPrefetchMode is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchModeDisabled. DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled ) @@ -144,14 +144,12 @@ type MountConfig struct { EnableHNS `yaml:"enable-hns"` FileSystemConfig `yaml:"file-system"` - // MetadataPrefetchMode indicates whether or not to prefetch the metadata of the mounted bucket. - // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. - // Supported values: - // "disabled": no pretch is executed. This is the default value, if not set. - // "synchronous": prefetch is executed with, blocking mount. - // "asynchronous": prefetch is executed without blocking mount. + // MetadataPrefetchMode indicates whether or not to prefetch the metadata of the mounted bucket at the time of mounting the bucket. + // Supported values: MetadataPrefetchModeDisabled, MetadataPrefetchModeSynchronous, and MetadataPrefetchModeAsynchronous. // Any other values will return error on mounting. - MetadataPrefetchMode MetadataPrefetchModeValue `yaml:"metadata-prefetch-mode"` + // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. This is because dynamic-mounts don't mount the bucket(s) at the time of + // gcsfuse command itself, which flag is targeted at. + MetadataPrefetchMode metadataPrefetchModeValue `yaml:"metadata-prefetch-mode"` } // LogRotateConfig defines the parameters for log rotation. It consists of three diff --git a/internal/config/yaml_parser.go b/internal/config/yaml_parser.go index 49408303d6..febbae3568 100644 --- a/internal/config/yaml_parser.go +++ b/internal/config/yaml_parser.go @@ -108,15 +108,12 @@ func (grpcClientConfig *GrpcClientConfig) validate() error { return nil } -func validateMetadataPrefetchMode(mode MetadataPrefetchModeValue) error { +func validateMetadataPrefetchMode(mode metadataPrefetchModeValue) error { switch mode { case MetadataPrefetchModeDisabled: fallthrough case MetadataPrefetchModeSynchronous: fallthrough - case MetadataPrefetchModeValue(""): - // this case is a temp-fix to avoid errors like: error parsing metadata-prefetch-mode: unsupported metadata-prefix-mode: \"\"; - fallthrough case MetadataPrefetchModeAsynchronous: return nil default: @@ -173,6 +170,11 @@ func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) { return mountConfig, fmt.Errorf("error parsing grpc-config: %w", err) } + // the following is a temp-fix to avoid errors like: error parsing metadata-prefetch-mode: unsupported metadata-prefix-mode: \"\"; + if mountConfig.MetadataPrefetchMode == metadataPrefetchModeValue("") { + mountConfig.MetadataPrefetchMode = DefaultMetadataPrefetchMode + } + if err = validateMetadataPrefetchMode(mountConfig.MetadataPrefetchMode); err != nil { return mountConfig, fmt.Errorf("error parsing metadata-prefetch-mode: %w", err) } From ed19a5d88f53051cf48b35a5b90f48bde835283b Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 09:38:26 +0000 Subject: [PATCH 11/36] add unit tests for populateList --- main_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/main_test.go b/main_test.go index 42a758902d..ad707f75ce 100644 --- a/main_test.go +++ b/main_test.go @@ -3,6 +3,7 @@ package main import ( "fmt" "os" + "path" "strings" "testing" @@ -248,3 +249,41 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false}" assert.Equal(t.T(), expected, actual) } + +func (t *MainTest) TestCallListRecursiveOnPopulatedDirectory() { + var err error + + rootdir, err := os.MkdirTemp("/tmp", "TestCallRecursive-*") + assert.False(t.T(), len(rootdir) == 0 || err != nil, "failed to create temp-directory for test. err = %v", err) + defer os.RemoveAll(rootdir) + + for i := 0; i < 10000; i++ { + file1, err := os.CreateTemp(rootdir, "*") + assert.False(t.T(), file1 == nil || err != nil, "failed to create file in \"%s\" for test. err = %v", rootdir, err) + } + + dirPath := rootdir + + for i := 0; i < 100; i++ { + dirPath = path.Join(dirPath, "dir") + err = os.Mkdir(dirPath, 0755) + assert.False(t.T(), err != nil, "failed to create sub-directory \"%s\" in \"%s\". err = %v", dirPath, rootdir, err) + + file, err := os.CreateTemp(dirPath, "*****") + assert.False(t.T(), file == nil || err != nil, "failed to create file in \"%s\" for test. err = %v", dirPath, err) + } + + err = callListRecursive(rootdir) + assert.Nil(t.T(), err) +} + +func (t *MainTest) TestCallListRecursiveOnUnpopulatedDirectory() { + var err error + + rootdir, err := os.MkdirTemp("/tmp", "TestCallRecursive-*") + assert.False(t.T(), len(rootdir) == 0 || err != nil, "failed to create temp-directory for test. err = %v", err) + defer os.RemoveAll(rootdir) + + err = callListRecursive(rootdir) + assert.Nil(t.T(), err) +} From db55591bb6df441d21c691bd5cc3f61b526a6f86 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 09:51:34 +0000 Subject: [PATCH 12/36] handle dynamic-mount scenario --- main.go | 78 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/main.go b/main.go index 8f4b8cef5b..7a8bbb3b15 100644 --- a/main.go +++ b/main.go @@ -274,6 +274,8 @@ func runCLIApp(c *cli.Context) (err error) { return } + isDynamicMount := bucketName == "" + logger.Infof("Start gcsfuse/%s for app %q using mount point: %s\n", getVersion(), flags.AppName, mountPoint) // Log mount-config and the CLI flags in the log-file. @@ -385,22 +387,26 @@ func runCLIApp(c *cli.Context) (err error) { err = fmt.Errorf("daemonize.Run: %w", err) return } else { - switch mountConfig.MetadataPrefetchMode { - case config.MetadataPrefetchModeDisabled: - logger.Infof(SuccessfulMountMessage) - case config.MetadataPrefetchModeSynchronous: - if err = callListRecursive(mountPoint); err != nil { - err = fmt.Errorf("daemonize.Run: metadata-prefetch failed: %w", err) - return - } - logger.Infof(SuccessfulMountMessage) - case config.MetadataPrefetchModeAsynchronous: + if isDynamicMount { logger.Infof(SuccessfulMountMessage) - go func() { - if err := callListRecursive(mountPoint); err != nil { - logger.Errorf("metadata-prefetch failed: %v", err) + } else { + switch mountConfig.MetadataPrefetchMode { + case config.MetadataPrefetchModeDisabled: + logger.Infof(SuccessfulMountMessage) + case config.MetadataPrefetchModeSynchronous: + if err = callListRecursive(mountPoint); err != nil { + err = fmt.Errorf("daemonize.Run: metadata-prefetch failed: %w", err) + return } - }() + logger.Infof(SuccessfulMountMessage) + case config.MetadataPrefetchModeAsynchronous: + logger.Infof(SuccessfulMountMessage) + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("metadata-prefetch failed: %v", err) + } + }() + } } } @@ -433,29 +439,33 @@ func runCLIApp(c *cli.Context) (err error) { } if err == nil { - switch mountConfig.MetadataPrefetchMode { - case config.MetadataPrefetchModeDisabled: + if isDynamicMount { markSuccessfullMount() - case config.MetadataPrefetchModeSynchronous: - if err = callListRecursive(mountPoint); err != nil { - // Printing via mountStatus will have duplicate logs on the console while - // mounting gcsfuse in foreground mode. But this is important to avoid - // losing error logs when run in the background mode. - logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) - err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) - callDaemonizeSignalOutcome(err) - return - } + } else { + switch mountConfig.MetadataPrefetchMode { + case config.MetadataPrefetchModeDisabled: + markSuccessfullMount() + case config.MetadataPrefetchModeSynchronous: + if err = callListRecursive(mountPoint); err != nil { + // Printing via mountStatus will have duplicate logs on the console while + // mounting gcsfuse in foreground mode. But this is important to avoid + // losing error logs when run in the background mode. + logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) + err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) + callDaemonizeSignalOutcome(err) + return + } - markSuccessfullMount() - case config.MetadataPrefetchModeAsynchronous: - markSuccessfullMount() + markSuccessfullMount() + case config.MetadataPrefetchModeAsynchronous: + markSuccessfullMount() - go func() { - if err := callListRecursive(mountPoint); err != nil { - logger.Errorf("Metadata-prefetch failed: %v", err) - } - }() + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("Metadata-prefetch failed: %v", err) + } + }() + } } } else { // Printing via mountStatus will have duplicate logs on the console while From 903c7e92ed750a74c79e27fcf07c7ab8a3792f2b Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 10:21:46 +0000 Subject: [PATCH 13/36] beautify test code --- main_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/main_test.go b/main_test.go index ad707f75ce..2fab744cfb 100644 --- a/main_test.go +++ b/main_test.go @@ -251,19 +251,15 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal } func (t *MainTest) TestCallListRecursiveOnPopulatedDirectory() { - var err error - + // Set up a mini file-system to test on. rootdir, err := os.MkdirTemp("/tmp", "TestCallRecursive-*") assert.False(t.T(), len(rootdir) == 0 || err != nil, "failed to create temp-directory for test. err = %v", err) defer os.RemoveAll(rootdir) - for i := 0; i < 10000; i++ { file1, err := os.CreateTemp(rootdir, "*") assert.False(t.T(), file1 == nil || err != nil, "failed to create file in \"%s\" for test. err = %v", rootdir, err) } - dirPath := rootdir - for i := 0; i < 100; i++ { dirPath = path.Join(dirPath, "dir") err = os.Mkdir(dirPath, 0755) @@ -273,17 +269,22 @@ func (t *MainTest) TestCallListRecursiveOnPopulatedDirectory() { assert.False(t.T(), file == nil || err != nil, "failed to create file in \"%s\" for test. err = %v", dirPath, err) } + // Call the target utility. err = callListRecursive(rootdir) + + // Test that it does not return error. assert.Nil(t.T(), err) } func (t *MainTest) TestCallListRecursiveOnUnpopulatedDirectory() { - var err error - + // Set up a mini file-system to test on. rootdir, err := os.MkdirTemp("/tmp", "TestCallRecursive-*") assert.False(t.T(), len(rootdir) == 0 || err != nil, "failed to create temp-directory for test. err = %v", err) defer os.RemoveAll(rootdir) + // Call the target utility. err = callListRecursive(rootdir) + + // Test that it does not return error. assert.Nil(t.T(), err) } From b7351e34dd45ef5f7c12406fe77ddaada9ec53ee Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 10:35:57 +0000 Subject: [PATCH 14/36] increase code-coverage --- .../testdata/metadata_prefetch_mode_supported_value4.yaml | 1 + internal/config/yaml_parser_test.go | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml new file mode 100644 index 0000000000..b3b98e6490 --- /dev/null +++ b/internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml @@ -0,0 +1 @@ +metadata-prefetch-mode: diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index f21ade3c1e..e55483c0c7 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -283,6 +283,10 @@ func (t *YamlParserTest) TestReadConfigFile_MetatadaPrefetchMode_SupportedValues filename: "testdata/metadata_prefetch_mode_supported_value3.yaml", mode: "disabled", }, + { + filename: "testdata/metadata_prefetch_mode_supported_value3.yaml", + mode: "disabled", + }, { filename: "testdata/metadata_prefetch_mode_unset.yaml", mode: "disabled", From 7f3887b6d7c42dfcb47df298e7e5727f7f3bc60f Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 10:48:14 +0000 Subject: [PATCH 15/36] add code for coverage --- main.go | 6 +++++- main_test.go | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 7a8bbb3b15..9760d7f778 100644 --- a/main.go +++ b/main.go @@ -216,7 +216,11 @@ func callListRecursive(mountPoint string) (err error) { err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { //logger.Infof("Walked path:%s, d=%s, isDir=%v", path, d.Name(), d.IsDir()) if err != nil { - return fmt.Errorf("got error walking: path=\"%s\", dentry=\"%s\", isDir=%v, err=%w", path, d.Name(), d.IsDir(), err) + if d == nil { + return fmt.Errorf("got error walking: path=\"%s\" does not exist, error = %w", path, err) + } else { + return fmt.Errorf("got error walking: path=\"%s\", dentry=\"%s\", isDir=%v, error = %w", path, d.Name(), d.IsDir(), err) + } } numItems++ diff --git a/main_test.go b/main_test.go index 2fab744cfb..3fdef8e3c5 100644 --- a/main_test.go +++ b/main_test.go @@ -288,3 +288,14 @@ func (t *MainTest) TestCallListRecursiveOnUnpopulatedDirectory() { // Test that it does not return error. assert.Nil(t.T(), err) } + +func (t *MainTest) TestCallListRecursiveOnNonExistingDirectory() { + // Set up a mini file-system to test on, which must fail. + rootdir := "/path/to/non/existing/directory" + + // Call the target utility. + err := callListRecursive(rootdir) + + // Test that it returns error. + assert.ErrorContains(t.T(), err, "does not exist") +} From b7da79aae4e348c354ccd65d92e8f16b305cc7c3 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Tue, 21 May 2024 11:07:55 +0000 Subject: [PATCH 16/36] fix typo in prev commit --- internal/config/yaml_parser_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index e55483c0c7..d481485c04 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -284,7 +284,7 @@ func (t *YamlParserTest) TestReadConfigFile_MetatadaPrefetchMode_SupportedValues mode: "disabled", }, { - filename: "testdata/metadata_prefetch_mode_supported_value3.yaml", + filename: "testdata/metadata_prefetch_mode_supported_value4.yaml", mode: "disabled", }, { From b8413c766da3c6cb80ef37b47bb3dd4e5ce9fdbe Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 06:25:46 +0000 Subject: [PATCH 17/36] remove type metadataPrefetchModeValue --- internal/config/mount_config.go | 12 ++++-------- internal/config/yaml_parser.go | 4 ++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 2a99ae7edb..9ff5d805ed 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -19,10 +19,6 @@ import ( "time" ) -// Special type declared to stop any random string -// from being set to MetadataPrefetchMode. -type metadataPrefetchModeValue string - const ( // Default log rotation config values. defaultMaxFileSizeMB = 512 @@ -56,11 +52,11 @@ const ( DefaultEnableHNS = false // MetadataPrefetchModeDisabled is the mode without metadata-prefetch. - MetadataPrefetchModeDisabled metadataPrefetchModeValue = "disabled" + MetadataPrefetchModeDisabled string = "disabled" // MetadataPrefetchModeSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. - MetadataPrefetchModeSynchronous metadataPrefetchModeValue = "synchronous" + MetadataPrefetchModeSynchronous string = "synchronous" // MetadataPrefetchModeAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. - MetadataPrefetchModeAsynchronous metadataPrefetchModeValue = "asynchronous" + MetadataPrefetchModeAsynchronous string = "asynchronous" // DefaultMetadataPrefetchMode is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchModeDisabled. DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled ) @@ -149,7 +145,7 @@ type MountConfig struct { // Any other values will return error on mounting. // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. This is because dynamic-mounts don't mount the bucket(s) at the time of // gcsfuse command itself, which flag is targeted at. - MetadataPrefetchMode metadataPrefetchModeValue `yaml:"metadata-prefetch-mode"` + MetadataPrefetchMode string `yaml:"metadata-prefetch-mode"` } // LogRotateConfig defines the parameters for log rotation. It consists of three diff --git a/internal/config/yaml_parser.go b/internal/config/yaml_parser.go index febbae3568..d290a5b533 100644 --- a/internal/config/yaml_parser.go +++ b/internal/config/yaml_parser.go @@ -108,7 +108,7 @@ func (grpcClientConfig *GrpcClientConfig) validate() error { return nil } -func validateMetadataPrefetchMode(mode metadataPrefetchModeValue) error { +func validateMetadataPrefetchMode(mode string) error { switch mode { case MetadataPrefetchModeDisabled: fallthrough @@ -171,7 +171,7 @@ func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) { } // the following is a temp-fix to avoid errors like: error parsing metadata-prefetch-mode: unsupported metadata-prefix-mode: \"\"; - if mountConfig.MetadataPrefetchMode == metadataPrefetchModeValue("") { + if mountConfig.MetadataPrefetchMode == "" { mountConfig.MetadataPrefetchMode = DefaultMetadataPrefetchMode } From 18dc77ed0c85908e8c40130567bef9d776204dec Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 06:26:03 +0000 Subject: [PATCH 18/36] address a review comment --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 9760d7f778..17f19ed848 100644 --- a/main.go +++ b/main.go @@ -229,7 +229,7 @@ func callListRecursive(mountPoint string) (err error) { logger.Debugf("... Completed recursive metadata-prefetch of directory: \"%s\". Number of items discovered: %v", mountPoint, numItems) - return + return nil } func runCLIApp(c *cli.Context) (err error) { From efd1234cf0d1900cff985f90f02f7ca5b6e61dc8 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 08:54:23 +0000 Subject: [PATCH 19/36] Add commandline flag metadata-prefetch-mode And switches main.go to call metadata-prefetch based on this flag from the mount-config flag of the same name. Also addresses some minor code comments. --- flags.go | 47 ++++++++++++++++++- flags_test.go | 47 +++++++++++++++++++ main.go | 7 ++- .../operations/operations_test.go | 13 +---- 4 files changed, 97 insertions(+), 17 deletions(-) diff --git a/flags.go b/flags.go index 2abe6746cf..d2304ee6ea 100644 --- a/flags.go +++ b/flags.go @@ -34,6 +34,10 @@ import ( const ( // maxSequentialReadSizeMb is the max value supported by sequential-read-size-mb flag. maxSequentialReadSizeMb = 1024 + + // MetadataPrefetchModeFlag is the name of the commandline flag for enabling + // metadata-prefetch mode aka 'ls -R' during mount. + MetadataPrefetchModeFlag = "metadata-prefetch-mode" ) // Set up custom help text for gcsfuse; in particular the usage section. @@ -360,6 +364,16 @@ func newApp() (app *cli.App) { Name: "debug_mutex", Usage: "Print debug messages when a mutex is held too long.", }, + + ///////////////////////// + // Post-mount actions + ///////////////////////// + + cli.StringFlag{ + Name: MetadataPrefetchModeFlag, + Value: config.DefaultMetadataPrefetchMode, + Usage: "This indicates whether or not to prefetch the metadata (prefilling of metadata caches and creation of inodes) of the mounted bucket at the time of mounting the bucket. Supported values: \"disabled\", \"synchronous\" and \"asynchronous\". Any other values will return error on mounting. This is applicable only to single-bucket mount-points, and not to dynamic-mount points.", + }, }, } @@ -423,6 +437,15 @@ type flagStorage struct { DebugHTTP bool DebugInvariants bool DebugMutex bool + + // Post-mount actions + + // MetadataPrefetchMode indicates whether or not to prefetch the metadata of the mounted bucket at the time of mounting the bucket. + // Supported values: MetadataPrefetchModeDisabled, MetadataPrefetchModeSynchronous, and MetadataPrefetchModeAsynchronous. + // Any other values will return error on mounting. + // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. This is because dynamic-mounts don't mount the bucket(s) at the time of + // gcsfuse command itself, which flag is targeted at. + MetadataPrefetchMode string } func resolveFilePath(filePath string, configKey string) (resolvedPath string, err error) { @@ -563,6 +586,9 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) { DebugFS: c.Bool("debug_fs"), DebugInvariants: c.Bool("debug_invariants"), DebugMutex: c.Bool("debug_mutex"), + + // Post-mount actions + MetadataPrefetchMode: c.String(MetadataPrefetchModeFlag), } // Handle the repeated "-o" flag. @@ -575,6 +601,19 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) { return } +func validateMetadataPrefetchMode(mode string) error { + switch mode { + case config.MetadataPrefetchModeDisabled: + fallthrough + case config.MetadataPrefetchModeSynchronous: + fallthrough + case config.MetadataPrefetchModeAsynchronous: + return nil + default: + return fmt.Errorf(config.UnsupportedMetadataPrefixModeError, mode) + } +} + func validateFlags(flags *flagStorage) (err error) { if flags.SequentialReadSizeMb < 1 || flags.SequentialReadSizeMb > maxSequentialReadSizeMb { err = fmt.Errorf("SequentialReadSizeMb should be less than %d", maxSequentialReadSizeMb) @@ -583,9 +622,15 @@ func validateFlags(flags *flagStorage) (err error) { if !flags.ClientProtocol.IsValid() { err = fmt.Errorf("client protocol: %s is not valid", flags.ClientProtocol) + return } - return + if err = validateMetadataPrefetchMode(flags.MetadataPrefetchMode); err != nil { + err = fmt.Errorf("%s: is not valid; error = %w", MetadataPrefetchModeFlag, err) + return + } + + return nil } // A cli.Generic that can be used with cli.GenericFlag to obtain an int flag diff --git a/flags_test.go b/flags_test.go index 957f1964bf..e413f2fca1 100644 --- a/flags_test.go +++ b/flags_test.go @@ -103,6 +103,9 @@ func (t *FlagsTest) Defaults() { ExpectFalse(f.DebugGCS) ExpectFalse(f.DebugHTTP) ExpectFalse(f.DebugInvariants) + + // Post-mount actions + ExpectEq(config.MetadataPrefetchModeDisabled, f.MetadataPrefetchMode) } func (t *FlagsTest) Bools() { @@ -213,6 +216,7 @@ func (t *FlagsTest) Strings() { "--temp-dir=foobar", "--only-dir=baz", "--client-protocol=HTTP2", + "--metadata-prefetch-mode=asynchronous", } f := parseArgs(args) @@ -220,6 +224,7 @@ func (t *FlagsTest) Strings() { ExpectEq("foobar", f.TempDir) ExpectEq("baz", f.OnlyDir) ExpectEq(mountpkg.HTTP2, f.ClientProtocol) + ExpectEq(config.MetadataPrefetchModeAsynchronous, f.MetadataPrefetchMode) } func (t *FlagsTest) Durations() { @@ -316,6 +321,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP1ClientPro flags := &flagStorage{ SequentialReadSizeMb: 10, ClientProtocol: mountpkg.ClientProtocol("http1"), + MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, } err := validateFlags(flags) @@ -327,6 +333,7 @@ func (t *FlagsTest) TestValidateFlagsForZeroSequentialReadSizeAndValidClientProt flags := &flagStorage{ SequentialReadSizeMb: 0, ClientProtocol: mountpkg.ClientProtocol("http2"), + MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, } err := validateFlags(flags) @@ -339,6 +346,7 @@ func (t *FlagsTest) TestValidateFlagsForSequentialReadSizeGreaterThan1024AndVali flags := &flagStorage{ SequentialReadSizeMb: 2048, ClientProtocol: mountpkg.ClientProtocol("http1"), + MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, } err := validateFlags(flags) @@ -351,6 +359,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndInValidClientP flags := &flagStorage{ SequentialReadSizeMb: 10, ClientProtocol: mountpkg.ClientProtocol("http4"), + MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, } err := validateFlags(flags) @@ -362,6 +371,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientPro flags := &flagStorage{ SequentialReadSizeMb: 10, ClientProtocol: mountpkg.ClientProtocol("http2"), + MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, } err := validateFlags(flags) @@ -369,6 +379,43 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientPro AssertEq(nil, err) } +func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { + for _, input := range []string{ + "disabled", "synchronous", "asynchronous", + } { + flags := &flagStorage{ + // Unrelated fields, not being tested here, so set to sane values. + SequentialReadSizeMb: 200, + ClientProtocol: mountpkg.ClientProtocol("http2"), + // The flag being tested. + MetadataPrefetchMode: input, + } + + err := validateFlags(flags) + + AssertEq(nil, err) + } +} + +func (t *FlagsTest) TestValidateFlagsForUnsupportedMetadataPrefetchMode() { + for _, input := range []string{ + "", "unsupported", + } { + flags := &flagStorage{ + // Unrelated fields, not being tested here, so set to sane values. + SequentialReadSizeMb: 200, + ClientProtocol: mountpkg.ClientProtocol("http2"), + // The flag being tested. + MetadataPrefetchMode: input, + } + + err := validateFlags(flags) + + AssertNe(nil, err) + AssertThat(err, Error(HasSubstr(fmt.Sprintf(config.UnsupportedMetadataPrefixModeError, input)))) + } +} + func (t *FlagsTest) Test_resolveConfigFilePaths() { mountConfig := &config.MountConfig{} mountConfig.LogConfig = config.LogConfig{ diff --git a/main.go b/main.go index 17f19ed848..c19ab53f55 100644 --- a/main.go +++ b/main.go @@ -214,7 +214,6 @@ func callListRecursive(mountPoint string) (err error) { logger.Debugf("Started recursive metadata-prefetch of directory: \"%s\" ...", mountPoint) numItems := 0 err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { - //logger.Infof("Walked path:%s, d=%s, isDir=%v", path, d.Name(), d.IsDir()) if err != nil { if d == nil { return fmt.Errorf("got error walking: path=\"%s\" does not exist, error = %w", path, err) @@ -394,12 +393,12 @@ func runCLIApp(c *cli.Context) (err error) { if isDynamicMount { logger.Infof(SuccessfulMountMessage) } else { - switch mountConfig.MetadataPrefetchMode { + switch flags.MetadataPrefetchMode { case config.MetadataPrefetchModeDisabled: logger.Infof(SuccessfulMountMessage) case config.MetadataPrefetchModeSynchronous: if err = callListRecursive(mountPoint); err != nil { - err = fmt.Errorf("daemonize.Run: metadata-prefetch failed: %w", err) + err = fmt.Errorf("metadata-prefetch failed: %w", err) return } logger.Infof(SuccessfulMountMessage) @@ -446,7 +445,7 @@ func runCLIApp(c *cli.Context) (err error) { if isDynamicMount { markSuccessfullMount() } else { - switch mountConfig.MetadataPrefetchMode { + switch flags.MetadataPrefetchMode { case config.MetadataPrefetchModeDisabled: markSuccessfullMount() case config.MetadataPrefetchModeSynchronous: diff --git a/tools/integration_tests/operations/operations_test.go b/tools/integration_tests/operations/operations_test.go index c0b0291286..f5a5ece137 100644 --- a/tools/integration_tests/operations/operations_test.go +++ b/tools/integration_tests/operations/operations_test.go @@ -141,18 +141,7 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { MetadataPrefetchMode: config.MetadataPrefetchModeAsynchronous, } filePath4 := setup.YAMLConfigFile(mountConfig4, "config4.yaml") - flags = append(flags, []string{"--config-file=" + filePath4}) - - // Set up config file testing metadata-prefetch-mode: "synchronous". - mountConfig5 := config.MountConfig{ - LogConfig: config.LogConfig{ - Severity: config.TRACE, - LogRotateConfig: config.DefaultLogRotateConfig(), - }, - MetadataPrefetchMode: config.MetadataPrefetchModeSynchronous, - } - filePath5 := setup.YAMLConfigFile(mountConfig5, "config5.yaml") - flags = append(flags, []string{"--config-file=" + filePath5}) + flags = append(flags, []string{"--config-file=" + filePath4, "--metadata-prefetch-mode=asynchronous"}) return flags } From 82fa82367b20c81a3321023edd297f9c228ef9c4 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 09:38:58 +0000 Subject: [PATCH 20/36] Put back unintentionally ignored error --- main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/main.go b/main.go index c19ab53f55..e10440b634 100644 --- a/main.go +++ b/main.go @@ -226,6 +226,10 @@ func callListRecursive(mountPoint string) (err error) { return err }) + if err != nil { + return fmt.Errorf("failed in recursive metadata-prefetch of directory: \"%s\"; error = %w", mountPoint, err) + } + logger.Debugf("... Completed recursive metadata-prefetch of directory: \"%s\". Number of items discovered: %v", mountPoint, numItems) return nil From 7f5d346f78ae0ec9869810b7a2bae95c711a5538 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 09:17:55 +0000 Subject: [PATCH 21/36] Migrate flags_test to stretchr/testify --- flags_test.go | 252 +++++++++++++++++++++++++------------------------- 1 file changed, 126 insertions(+), 126 deletions(-) diff --git a/flags_test.go b/flags_test.go index e413f2fca1..dd388bd172 100644 --- a/flags_test.go +++ b/flags_test.go @@ -25,36 +25,35 @@ import ( "github.com/googlecloudplatform/gcsfuse/v2/internal/config" "github.com/googlecloudplatform/gcsfuse/v2/internal/mount" mountpkg "github.com/googlecloudplatform/gcsfuse/v2/internal/mount" - . "github.com/jacobsa/oglematchers" - . "github.com/jacobsa/ogletest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" "github.com/urfave/cli" ) -func TestFlags(t *testing.T) { RunTests(t) } +func TestFlags(t *testing.T) { suite.Run(t, new(FlagsTest)) } //////////////////////////////////////////////////////////////////////// // Boilerplate //////////////////////////////////////////////////////////////////////// type FlagsTest struct { + suite.Suite } -func init() { RegisterTestSuite(&FlagsTest{}) } - -func parseArgs(args []string) (flags *flagStorage) { +func parseArgs(t *FlagsTest, args []string) (flags *flagStorage) { // Create a CLI app, and abuse it to snoop on the flags. app := newApp() var err error app.Action = func(appCtx *cli.Context) { flags, err = populateFlags(appCtx) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) } // Simulate argv. fullArgs := append([]string{"some_app"}, args...) err = app.Run(fullArgs) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) return } @@ -64,48 +63,48 @@ func parseArgs(args []string) (flags *flagStorage) { //////////////////////////////////////////////////////////////////////// func (t *FlagsTest) Defaults() { - f := parseArgs([]string{}) + f := parseArgs(t, []string{}) // File system - ExpectNe(nil, f.MountOptions) - ExpectEq(0, len(f.MountOptions), "Options: %v", f.MountOptions) + assert.NotEqual(t.T(), nil, f.MountOptions) + assert.Equal(t.T(), 0, len(f.MountOptions), "Options: %v", f.MountOptions) - ExpectEq(os.FileMode(0755), f.DirMode) - ExpectEq(os.FileMode(0644), f.FileMode) - ExpectEq(-1, f.Uid) - ExpectEq(-1, f.Gid) - ExpectFalse(f.ImplicitDirs) - ExpectFalse(f.IgnoreInterrupts) + assert.Equal(t.T(), os.FileMode(0755), f.DirMode) + assert.Equal(t.T(), os.FileMode(0644), f.FileMode) + assert.Equal(t.T(), -1, f.Uid) + assert.Equal(t.T(), -1, f.Gid) + assert.False(t.T(), f.ImplicitDirs) + assert.False(t.T(), f.IgnoreInterrupts) // GCS - ExpectEq("", f.KeyFile) - ExpectEq(-1, f.EgressBandwidthLimitBytesPerSecond) - ExpectEq(-1, f.OpRateLimitHz) - ExpectTrue(f.ReuseTokenFromUrl) - ExpectEq(nil, f.CustomEndpoint) - ExpectFalse(f.AnonymousAccess) + assert.Equal(t.T(), "", f.KeyFile) + assert.Equal(t.T(), -1, f.EgressBandwidthLimitBytesPerSecond) + assert.Equal(t.T(), -1, f.OpRateLimitHz) + assert.True(t.T(), f.ReuseTokenFromUrl) + assert.Equal(t.T(), nil, f.CustomEndpoint) + assert.False(t.T(), f.AnonymousAccess) // Tuning - ExpectEq(mount.DefaultStatCacheCapacity, f.StatCacheCapacity) - ExpectEq(mount.DefaultStatOrTypeCacheTTL, f.StatCacheTTL) - ExpectEq(mount.DefaultStatOrTypeCacheTTL, f.TypeCacheTTL) - ExpectEq(0, f.HttpClientTimeout) - ExpectEq("", f.TempDir) - ExpectEq(2, f.RetryMultiplier) - ExpectFalse(f.EnableNonexistentTypeCache) - ExpectEq(0, f.MaxConnsPerHost) + assert.Equal(t.T(), mount.DefaultStatCacheCapacity, f.StatCacheCapacity) + assert.Equal(t.T(), mount.DefaultStatOrTypeCacheTTL, f.StatCacheTTL) + assert.Equal(t.T(), mount.DefaultStatOrTypeCacheTTL, f.TypeCacheTTL) + assert.Equal(t.T(), 0, f.HttpClientTimeout) + assert.Equal(t.T(), "", f.TempDir) + assert.Equal(t.T(), 2, f.RetryMultiplier) + assert.False(t.T(), f.EnableNonexistentTypeCache) + assert.Equal(t.T(), 0, f.MaxConnsPerHost) // Logging - ExpectTrue(f.DebugFuseErrors) + assert.True(t.T(), f.DebugFuseErrors) // Debugging - ExpectFalse(f.DebugFuse) - ExpectFalse(f.DebugGCS) - ExpectFalse(f.DebugHTTP) - ExpectFalse(f.DebugInvariants) + assert.False(t.T(), f.DebugFuse) + assert.False(t.T(), f.DebugGCS) + assert.False(t.T(), f.DebugHTTP) + assert.False(t.T(), f.DebugInvariants) // Post-mount actions - ExpectEq(config.MetadataPrefetchModeDisabled, f.MetadataPrefetchMode) + assert.Equal(t.T(), config.MetadataPrefetchModeDisabled, f.MetadataPrefetchMode) } func (t *FlagsTest) Bools() { @@ -132,18 +131,18 @@ func (t *FlagsTest) Bools() { args = append(args, fmt.Sprintf("-%s", n)) } - f = parseArgs(args) - ExpectTrue(f.ImplicitDirs) - ExpectTrue(f.ReuseTokenFromUrl) - ExpectTrue(f.DebugFuseErrors) - ExpectTrue(f.DebugFuse) - ExpectTrue(f.DebugGCS) - ExpectTrue(f.DebugHTTP) - ExpectTrue(f.DebugInvariants) - ExpectTrue(f.EnableNonexistentTypeCache) - ExpectTrue(f.ExperimentalEnableJsonRead) - ExpectTrue(f.IgnoreInterrupts) - ExpectTrue(f.AnonymousAccess) + f = parseArgs(t, args) + assert.True(t.T(), f.ImplicitDirs) + assert.True(t.T(), f.ReuseTokenFromUrl) + assert.True(t.T(), f.DebugFuseErrors) + assert.True(t.T(), f.DebugFuse) + assert.True(t.T(), f.DebugGCS) + assert.True(t.T(), f.DebugHTTP) + assert.True(t.T(), f.DebugInvariants) + assert.True(t.T(), f.EnableNonexistentTypeCache) + assert.True(t.T(), f.ExperimentalEnableJsonRead) + assert.True(t.T(), f.IgnoreInterrupts) + assert.True(t.T(), f.AnonymousAccess) // --foo=false form args = nil @@ -151,15 +150,15 @@ func (t *FlagsTest) Bools() { args = append(args, fmt.Sprintf("-%s=false", n)) } - f = parseArgs(args) - ExpectFalse(f.ImplicitDirs) - ExpectFalse(f.ReuseTokenFromUrl) - ExpectFalse(f.DebugFuseErrors) - ExpectFalse(f.DebugFuse) - ExpectFalse(f.DebugGCS) - ExpectFalse(f.DebugHTTP) - ExpectFalse(f.DebugInvariants) - ExpectFalse(f.EnableNonexistentTypeCache) + f = parseArgs(t, args) + assert.False(t.T(), f.ImplicitDirs) + assert.False(t.T(), f.ReuseTokenFromUrl) + assert.False(t.T(), f.DebugFuseErrors) + assert.False(t.T(), f.DebugFuse) + assert.False(t.T(), f.DebugGCS) + assert.False(t.T(), f.DebugHTTP) + assert.False(t.T(), f.DebugInvariants) + assert.False(t.T(), f.EnableNonexistentTypeCache) // --foo=true form args = nil @@ -167,15 +166,15 @@ func (t *FlagsTest) Bools() { args = append(args, fmt.Sprintf("-%s=true", n)) } - f = parseArgs(args) - ExpectTrue(f.ImplicitDirs) - ExpectTrue(f.ReuseTokenFromUrl) - ExpectTrue(f.DebugFuseErrors) - ExpectTrue(f.DebugFuse) - ExpectTrue(f.DebugGCS) - ExpectTrue(f.DebugHTTP) - ExpectTrue(f.DebugInvariants) - ExpectTrue(f.EnableNonexistentTypeCache) + f = parseArgs(t, args) + assert.True(t.T(), f.ImplicitDirs) + assert.True(t.T(), f.ReuseTokenFromUrl) + assert.True(t.T(), f.DebugFuseErrors) + assert.True(t.T(), f.DebugFuse) + assert.True(t.T(), f.DebugGCS) + assert.True(t.T(), f.DebugHTTP) + assert.True(t.T(), f.DebugInvariants) + assert.True(t.T(), f.EnableNonexistentTypeCache) } func (t *FlagsTest) DecimalNumbers() { @@ -189,14 +188,14 @@ func (t *FlagsTest) DecimalNumbers() { "--max-conns-per-host=100", } - f := parseArgs(args) - ExpectEq(17, f.Uid) - ExpectEq(19, f.Gid) - ExpectEq(123.4, f.EgressBandwidthLimitBytesPerSecond) - ExpectEq(56.78, f.OpRateLimitHz) - ExpectEq(8192, f.StatCacheCapacity) - ExpectEq(100, f.MaxIdleConnsPerHost) - ExpectEq(100, f.MaxConnsPerHost) + f := parseArgs(t, args) + assert.Equal(t.T(), 17, f.Uid) + assert.Equal(t.T(), 19, f.Gid) + assert.Equal(t.T(), 123.4, f.EgressBandwidthLimitBytesPerSecond) + assert.Equal(t.T(), 56.78, f.OpRateLimitHz) + assert.Equal(t.T(), 8192, f.StatCacheCapacity) + assert.Equal(t.T(), 100, f.MaxIdleConnsPerHost) + assert.Equal(t.T(), 100, f.MaxConnsPerHost) } func (t *FlagsTest) OctalNumbers() { @@ -205,9 +204,9 @@ func (t *FlagsTest) OctalNumbers() { "--file-mode", "611", } - f := parseArgs(args) - ExpectEq(os.FileMode(0711), f.DirMode) - ExpectEq(os.FileMode(0611), f.FileMode) + f := parseArgs(t, args) + assert.Equal(t.T(), os.FileMode(0711), f.DirMode) + assert.Equal(t.T(), os.FileMode(0611), f.FileMode) } func (t *FlagsTest) Strings() { @@ -219,12 +218,12 @@ func (t *FlagsTest) Strings() { "--metadata-prefetch-mode=asynchronous", } - f := parseArgs(args) - ExpectEq("-asdf", f.KeyFile) - ExpectEq("foobar", f.TempDir) - ExpectEq("baz", f.OnlyDir) - ExpectEq(mountpkg.HTTP2, f.ClientProtocol) - ExpectEq(config.MetadataPrefetchModeAsynchronous, f.MetadataPrefetchMode) + f := parseArgs(t, args) + assert.Equal(t.T(), "-asdf", f.KeyFile) + assert.Equal(t.T(), "foobar", f.TempDir) + assert.Equal(t.T(), "baz", f.OnlyDir) + assert.Equal(t.T(), mountpkg.HTTP2, f.ClientProtocol) + assert.Equal(t.T(), config.MetadataPrefetchModeAsynchronous, f.MetadataPrefetchMode) } func (t *FlagsTest) Durations() { @@ -236,12 +235,12 @@ func (t *FlagsTest) Durations() { "--max-retry-sleep", "30s", } - f := parseArgs(args) - ExpectEq(time.Minute+17*time.Second+100*time.Millisecond, f.StatCacheTTL) - ExpectEq(50*time.Second+900*time.Millisecond, f.TypeCacheTTL) - ExpectEq(800*time.Millisecond, f.HttpClientTimeout) - ExpectEq(-1*time.Second, f.MaxRetryDuration) - ExpectEq(30*time.Second, f.MaxRetrySleep) + f := parseArgs(t, args) + assert.Equal(t.T(), time.Minute+17*time.Second+100*time.Millisecond, f.StatCacheTTL) + assert.Equal(t.T(), 50*time.Second+900*time.Millisecond, f.TypeCacheTTL) + assert.Equal(t.T(), 800*time.Millisecond, f.HttpClientTimeout) + assert.Equal(t.T(), -1*time.Second, f.MaxRetryDuration) + assert.Equal(t.T(), 30*time.Second, f.MaxRetrySleep) } func (t *FlagsTest) Maps() { @@ -250,7 +249,7 @@ func (t *FlagsTest) Maps() { "-o", "user=jacobsa,noauto", } - f := parseArgs(args) + f := parseArgs(t, args) var keys sort.StringSlice for k := range f.MountOptions { @@ -258,31 +257,32 @@ func (t *FlagsTest) Maps() { } sort.Sort(keys) - AssertThat(keys, ElementsAre("noauto", "nodev", "rw", "user")) + assert.ElementsMatch(t.T(), + keys, []string{"noauto", "nodev", "rw", "user"}) - ExpectEq("", f.MountOptions["noauto"]) - ExpectEq("", f.MountOptions["nodev"]) - ExpectEq("", f.MountOptions["rw"]) - ExpectEq("jacobsa", f.MountOptions["user"]) + assert.Equal(t.T(), "", f.MountOptions["noauto"]) + assert.Equal(t.T(), "", f.MountOptions["nodev"]) + assert.Equal(t.T(), "", f.MountOptions["rw"]) + assert.Equal(t.T(), "jacobsa", f.MountOptions["user"]) } func (t *FlagsTest) TestResolvePathForTheFlagInContext() { app := newApp() currentWorkingDir, err := os.Getwd() - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) app.Action = func(appCtx *cli.Context) { err = resolvePathForTheFlagInContext("log-file", appCtx) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) err = resolvePathForTheFlagInContext("key-file", appCtx) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) err = resolvePathForTheFlagInContext("config-file", appCtx) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) - ExpectEq(filepath.Join(currentWorkingDir, "test.txt"), + assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"), appCtx.String("log-file")) - ExpectEq(filepath.Join(currentWorkingDir, "test.txt"), + assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"), appCtx.String("key-file")) - ExpectEq(filepath.Join(currentWorkingDir, "config.yaml"), + assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"), appCtx.String("config-file")) } // Simulate argv. @@ -291,21 +291,21 @@ func (t *FlagsTest) TestResolvePathForTheFlagInContext() { err = app.Run(fullArgs) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) } func (t *FlagsTest) TestResolvePathForTheFlagsInContext() { app := newApp() currentWorkingDir, err := os.Getwd() - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) app.Action = func(appCtx *cli.Context) { resolvePathForTheFlagsInContext(appCtx) - ExpectEq(filepath.Join(currentWorkingDir, "test.txt"), + assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"), appCtx.String("log-file")) - ExpectEq(filepath.Join(currentWorkingDir, "test.txt"), + assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"), appCtx.String("key-file")) - ExpectEq(filepath.Join(currentWorkingDir, "config.yaml"), + assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"), appCtx.String("config-file")) } // Simulate argv. @@ -314,7 +314,7 @@ func (t *FlagsTest) TestResolvePathForTheFlagsInContext() { err = app.Run(fullArgs) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) } func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP1ClientProtocol() { @@ -326,7 +326,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP1ClientPro err := validateFlags(flags) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) } func (t *FlagsTest) TestValidateFlagsForZeroSequentialReadSizeAndValidClientProtocol() { @@ -338,8 +338,8 @@ func (t *FlagsTest) TestValidateFlagsForZeroSequentialReadSizeAndValidClientProt err := validateFlags(flags) - AssertNe(nil, err) - AssertEq("SequentialReadSizeMb should be less than 1024", err.Error()) + assert.NotEqual(t.T(), nil, err) + assert.Equal(t.T(), "SequentialReadSizeMb should be less than 1024", err.Error()) } func (t *FlagsTest) TestValidateFlagsForSequentialReadSizeGreaterThan1024AndValidClientProtocol() { @@ -351,8 +351,8 @@ func (t *FlagsTest) TestValidateFlagsForSequentialReadSizeGreaterThan1024AndVali err := validateFlags(flags) - AssertNe(nil, err) - AssertEq("SequentialReadSizeMb should be less than 1024", err.Error()) + assert.NotEqual(t.T(), nil, err) + assert.Equal(t.T(), "SequentialReadSizeMb should be less than 1024", err.Error()) } func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndInValidClientProtocol() { @@ -364,7 +364,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndInValidClientP err := validateFlags(flags) - AssertEq("client protocol: http4 is not valid", err.Error()) + assert.Equal(t.T(), "client protocol: http4 is not valid", err.Error()) } func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientProtocol() { @@ -376,7 +376,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientPro err := validateFlags(flags) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) } func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { @@ -393,7 +393,7 @@ func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { err := validateFlags(flags) - AssertEq(nil, err) + assert.Equal(t.T(), nil, err) } } @@ -411,8 +411,8 @@ func (t *FlagsTest) TestValidateFlagsForUnsupportedMetadataPrefetchMode() { err := validateFlags(flags) - AssertNe(nil, err) - AssertThat(err, Error(HasSubstr(fmt.Sprintf(config.UnsupportedMetadataPrefixModeError, input)))) + assert.NotEqual(t.T(), nil, err) + assert.ErrorContains(t.T(), err, fmt.Sprintf(config.UnsupportedMetadataPrefixModeError, input)) } } @@ -425,11 +425,11 @@ func (t *FlagsTest) Test_resolveConfigFilePaths() { err := resolveConfigFilePaths(mountConfig) - AssertEq(nil, err) + assert.Equal(t.T(), 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) + assert.Equal(t.T(), nil, err) + assert.Equal(t.T(), filepath.Join(homeDir, "test.txt"), mountConfig.LogConfig.FilePath) + assert.EqualValues(t.T(), filepath.Join(homeDir, "cache-dir"), mountConfig.CacheDir) } func (t *FlagsTest) Test_resolveConfigFilePaths_WithoutSettingPaths() { @@ -437,7 +437,7 @@ func (t *FlagsTest) Test_resolveConfigFilePaths_WithoutSettingPaths() { err := resolveConfigFilePaths(mountConfig) - AssertEq(nil, err) - ExpectEq("", mountConfig.LogConfig.FilePath) - ExpectEq("", mountConfig.CacheDir) + assert.Equal(t.T(), nil, err) + assert.Equal(t.T(), "", mountConfig.LogConfig.FilePath) + assert.EqualValues(t.T(), "", mountConfig.CacheDir) } From ab9c962710f2487d1a245452b06d74a1de36aac1 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 09:33:08 +0000 Subject: [PATCH 22/36] remove config-flag metadata-prefetch-mode --- internal/config/mount_config.go | 8 ---- ...tadata_prefetch_mode_supported_value1.yaml | 1 - ...tadata_prefetch_mode_supported_value2.yaml | 1 - ...tadata_prefetch_mode_supported_value3.yaml | 1 - ...tadata_prefetch_mode_supported_value4.yaml | 1 - .../metadata_prefetch_mode_unset.yaml | 0 ...adata_prefetch_mode_unsupported_value.yaml | 1 - internal/config/yaml_parser.go | 22 --------- internal/config/yaml_parser_test.go | 47 ------------------- main_test.go | 42 ++--------------- .../operations/operations_test.go | 1 - 11 files changed, 3 insertions(+), 122 deletions(-) delete mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml delete mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml delete mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml delete mode 100644 internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml delete mode 100644 internal/config/testdata/metadata_prefetch_mode_unset.yaml delete mode 100644 internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 9ff5d805ed..54a6e39201 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -139,13 +139,6 @@ type MountConfig struct { AuthConfig `yaml:"auth-config"` EnableHNS `yaml:"enable-hns"` FileSystemConfig `yaml:"file-system"` - - // MetadataPrefetchMode indicates whether or not to prefetch the metadata of the mounted bucket at the time of mounting the bucket. - // Supported values: MetadataPrefetchModeDisabled, MetadataPrefetchModeSynchronous, and MetadataPrefetchModeAsynchronous. - // Any other values will return error on mounting. - // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. This is because dynamic-mounts don't mount the bucket(s) at the time of - // gcsfuse command itself, which flag is targeted at. - MetadataPrefetchMode string `yaml:"metadata-prefetch-mode"` } // LogRotateConfig defines the parameters for log rotation. It consists of three @@ -197,6 +190,5 @@ func NewMountConfig() *MountConfig { AnonymousAccess: DefaultAnonymousAccess, } mountConfig.EnableHNS = DefaultEnableHNS - mountConfig.MetadataPrefetchMode = DefaultMetadataPrefetchMode return mountConfig } diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml deleted file mode 100644 index 36d892de2e..0000000000 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value1.yaml +++ /dev/null @@ -1 +0,0 @@ -metadata-prefetch-mode: synchronous diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml deleted file mode 100644 index 376808e6b5..0000000000 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value2.yaml +++ /dev/null @@ -1 +0,0 @@ -metadata-prefetch-mode: asynchronous diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml deleted file mode 100644 index 021b566f86..0000000000 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value3.yaml +++ /dev/null @@ -1 +0,0 @@ -metadata-prefetch-mode: disabled diff --git a/internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml b/internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml deleted file mode 100644 index b3b98e6490..0000000000 --- a/internal/config/testdata/metadata_prefetch_mode_supported_value4.yaml +++ /dev/null @@ -1 +0,0 @@ -metadata-prefetch-mode: diff --git a/internal/config/testdata/metadata_prefetch_mode_unset.yaml b/internal/config/testdata/metadata_prefetch_mode_unset.yaml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml b/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml deleted file mode 100644 index 0e2b7b7b89..0000000000 --- a/internal/config/testdata/metadata_prefetch_mode_unsupported_value.yaml +++ /dev/null @@ -1 +0,0 @@ -metadata-prefetch-mode: unsupported diff --git a/internal/config/yaml_parser.go b/internal/config/yaml_parser.go index d290a5b533..87dd73f25a 100644 --- a/internal/config/yaml_parser.go +++ b/internal/config/yaml_parser.go @@ -108,19 +108,6 @@ func (grpcClientConfig *GrpcClientConfig) validate() error { return nil } -func validateMetadataPrefetchMode(mode string) error { - switch mode { - case MetadataPrefetchModeDisabled: - fallthrough - case MetadataPrefetchModeSynchronous: - fallthrough - case MetadataPrefetchModeAsynchronous: - return nil - default: - return fmt.Errorf(UnsupportedMetadataPrefixModeError, mode) - } -} - func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) { mountConfig = NewMountConfig() @@ -170,14 +157,5 @@ func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) { return mountConfig, fmt.Errorf("error parsing grpc-config: %w", err) } - // the following is a temp-fix to avoid errors like: error parsing metadata-prefetch-mode: unsupported metadata-prefix-mode: \"\"; - if mountConfig.MetadataPrefetchMode == "" { - mountConfig.MetadataPrefetchMode = DefaultMetadataPrefetchMode - } - - if err = validateMetadataPrefetchMode(mountConfig.MetadataPrefetchMode); err != nil { - return mountConfig, fmt.Errorf("error parsing metadata-prefetch-mode: %w", err) - } - return } diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index d481485c04..ac667140d2 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -128,9 +128,6 @@ func (t *YamlParserTest) TestReadConfigFile_ValidConfig() { // file-system config assert.True(t.T(), mountConfig.FileSystemConfig.IgnoreInterrupts) assert.True(t.T(), mountConfig.FileSystemConfig.DisableParallelDirops) - - // metadata-prefetch-mode config - assert.Equal(t.T(), "disabled", string(mountConfig.MetadataPrefetchMode)) } func (t *YamlParserTest) TestReadConfigFile_InvalidLogConfig() { @@ -266,47 +263,3 @@ func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetDisableParalle assert.NotNil(t.T(), mountConfig) assert.False(t.T(), mountConfig.FileSystemConfig.DisableParallelDirops) } - -func (t *YamlParserTest) TestReadConfigFile_MetatadaPrefetchMode_SupportedValues() { - for _, input := range []struct { - filename string - mode string - }{{ - filename: "testdata/metadata_prefetch_mode_supported_value1.yaml", - mode: "synchronous", - }, - { - filename: "testdata/metadata_prefetch_mode_supported_value2.yaml", - mode: "asynchronous", - }, - { - filename: "testdata/metadata_prefetch_mode_supported_value3.yaml", - mode: "disabled", - }, - { - filename: "testdata/metadata_prefetch_mode_supported_value4.yaml", - mode: "disabled", - }, - { - filename: "testdata/metadata_prefetch_mode_unset.yaml", - mode: "disabled", - }, - } { - mountConfig, err := ParseConfigFile(input.filename) - assert.NoError(t.T(), err) - assert.NotNil(t.T(), mountConfig) - assert.Equal(t.T(), input.mode, string(mountConfig.MetadataPrefetchMode)) - } -} - -func (t *YamlParserTest) TestReadConfigFile_MetatadaPrefetchMode_UnsupportedValues() { - for _, input := range []struct { - filename string - }{{ - filename: "testdata/metadata_prefetch_mode_unsupported_value.yaml", - }, - } { - _, err := ParseConfigFile(input.filename) - assert.ErrorContains(t.T(), err, fmt.Sprintf(UnsupportedMetadataPrefixModeError, "unsupported")) - } -} diff --git a/main_test.go b/main_test.go index 3fdef8e3c5..d0c3a2ecb7 100644 --- a/main_test.go +++ b/main_test.go @@ -179,7 +179,7 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInMountConfigAsMarshal actual, err := util.Stringify(mountConfig) assert.Equal(t.T(), nil, err) - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"TRACE\",\"Format\":\"\",\"FilePath\":\"\\\"path\\\"to\\\"file\\\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":2,\"BackupFileCount\":2,\"Compress\":true},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":true,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"\"}" + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"TRACE\",\"Format\":\"\",\"FilePath\":\"\\\"path\\\"to\\\"file\\\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":2,\"BackupFileCount\":2,\"Compress\":true},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":true,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false}" assert.Equal(t.T(), expected, actual) } @@ -191,43 +191,7 @@ func (t *MainTest) TestEnableHNSFlagFalse() { actual, err := util.Stringify(mountConfig) assert.Equal(t.T(), nil, err) - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"\"}" - assert.Equal(t.T(), expected, actual) -} - -func (t *MainTest) TestMetadataPrefetchModeSynchronous() { - mountConfig := &config.MountConfig{ - MetadataPrefetchMode: config.MetadataPrefetchModeSynchronous, - } - - actual, err := util.Stringify(mountConfig) - assert.Equal(t.T(), nil, err) - - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"synchronous\"}" - assert.Equal(t.T(), expected, actual) -} - -func (t *MainTest) TestMetadataPrefetchModeAsynchronous() { - mountConfig := &config.MountConfig{ - MetadataPrefetchMode: config.MetadataPrefetchModeAsynchronous, - } - - actual, err := util.Stringify(mountConfig) - assert.Equal(t.T(), nil, err) - - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"asynchronous\"}" - assert.Equal(t.T(), expected, actual) -} - -func (t *MainTest) TestMetadataPrefetchModeDisabled() { - mountConfig := &config.MountConfig{ - MetadataPrefetchMode: config.MetadataPrefetchModeDisabled, - } - - actual, err := util.Stringify(mountConfig) - assert.Equal(t.T(), nil, err) - - expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false,\"MetadataPrefetchMode\":\"disabled\"}" + expected := "{\"CreateEmptyFile\":false,\"Severity\":\"\",\"Format\":\"\",\"FilePath\":\"\",\"LogRotateConfig\":{\"MaxFileSizeMB\":0,\"BackupFileCount\":0,\"Compress\":false},\"MaxSizeMB\":0,\"CacheFileForRangeRead\":false,\"CacheDir\":\"\",\"TtlInSeconds\":0,\"TypeCacheMaxSizeMB\":0,\"StatCacheMaxSizeMB\":0,\"EnableEmptyManagedFolders\":false,\"ConnPoolSize\":0,\"AnonymousAccess\":false,\"EnableHNS\":false,\"IgnoreInterrupts\":false,\"DisableParallelDirops\":false}" assert.Equal(t.T(), expected, actual) } @@ -246,7 +210,7 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal actual, err := util.Stringify(flags) assert.Equal(t.T(), nil, err) - expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false}" + expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false,\"MetadataPrefetchMode\":\"\"}" assert.Equal(t.T(), expected, actual) } diff --git a/tools/integration_tests/operations/operations_test.go b/tools/integration_tests/operations/operations_test.go index f5a5ece137..e868ebafd0 100644 --- a/tools/integration_tests/operations/operations_test.go +++ b/tools/integration_tests/operations/operations_test.go @@ -138,7 +138,6 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { Severity: config.TRACE, LogRotateConfig: config.DefaultLogRotateConfig(), }, - MetadataPrefetchMode: config.MetadataPrefetchModeAsynchronous, } filePath4 := setup.YAMLConfigFile(mountConfig4, "config4.yaml") flags = append(flags, []string{"--config-file=" + filePath4, "--metadata-prefetch-mode=asynchronous"}) From 7eb076f736fffb5f5a4484b754ab65b74ddd864b Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 11:36:01 +0000 Subject: [PATCH 23/36] fix for persistent mounting --- tools/mount_gcsfuse/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index 7ef538f7a9..80f0c07a9e 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -121,7 +121,8 @@ func makeGcsfuseArgs( "log_format", "log_file", "custom_endpoint", - "config_file": + "config_file", + "metadata_prefetch_mode": args = append(args, "--"+strings.Replace(name, "_", "-", -1), value) // Special case: support mount-like formatting for gcsfuse debug flags. From 427abfe7ff79610c52a26122763b09aab5e9a4d1 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 11:58:43 +0000 Subject: [PATCH 24/36] address couple of review comments --- flags.go | 9 +++------ main.go | 10 +++++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/flags.go b/flags.go index d2304ee6ea..5330e66075 100644 --- a/flags.go +++ b/flags.go @@ -616,18 +616,15 @@ func validateMetadataPrefetchMode(mode string) error { func validateFlags(flags *flagStorage) (err error) { if flags.SequentialReadSizeMb < 1 || flags.SequentialReadSizeMb > maxSequentialReadSizeMb { - err = fmt.Errorf("SequentialReadSizeMb should be less than %d", maxSequentialReadSizeMb) - return + return fmt.Errorf("SequentialReadSizeMb should be less than %d", maxSequentialReadSizeMb) } if !flags.ClientProtocol.IsValid() { - err = fmt.Errorf("client protocol: %s is not valid", flags.ClientProtocol) - return + return fmt.Errorf("client protocol: %s is not valid", flags.ClientProtocol) } if err = validateMetadataPrefetchMode(flags.MetadataPrefetchMode); err != nil { - err = fmt.Errorf("%s: is not valid; error = %w", MetadataPrefetchModeFlag, err) - return + return fmt.Errorf("%s: is not valid; error = %w", MetadataPrefetchModeFlag, err) } return nil diff --git a/main.go b/main.go index e10440b634..809b7f0c05 100644 --- a/main.go +++ b/main.go @@ -439,7 +439,7 @@ func runCLIApp(c *cli.Context) (err error) { } } - markSuccessfullMount := func() { + markSuccessfulMount := func() { // Print the success message in the log-file/stdout depending on what the logger is set to. logger.Info(SuccessfulMountMessage) callDaemonizeSignalOutcome(nil) @@ -447,11 +447,11 @@ func runCLIApp(c *cli.Context) (err error) { if err == nil { if isDynamicMount { - markSuccessfullMount() + markSuccessfulMount() } else { switch flags.MetadataPrefetchMode { case config.MetadataPrefetchModeDisabled: - markSuccessfullMount() + markSuccessfulMount() case config.MetadataPrefetchModeSynchronous: if err = callListRecursive(mountPoint); err != nil { // Printing via mountStatus will have duplicate logs on the console while @@ -463,9 +463,9 @@ func runCLIApp(c *cli.Context) (err error) { return } - markSuccessfullMount() + markSuccessfulMount() case config.MetadataPrefetchModeAsynchronous: - markSuccessfullMount() + markSuccessfulMount() go func() { if err := callListRecursive(mountPoint); err != nil { From 2b64ed125c2409c2bba357627138cfc4dc1ed8d8 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 13:11:48 +0000 Subject: [PATCH 25/36] addressed some more comments --- main_test.go | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/main_test.go b/main_test.go index d0c3a2ecb7..6ca9c9c389 100644 --- a/main_test.go +++ b/main_test.go @@ -3,7 +3,6 @@ package main import ( "fmt" "os" - "path" "strings" "testing" @@ -214,40 +213,14 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal assert.Equal(t.T(), expected, actual) } -func (t *MainTest) TestCallListRecursiveOnPopulatedDirectory() { +func (t *MainTest) TestCallListRecursiveOnExistingDirectory() { // Set up a mini file-system to test on. - rootdir, err := os.MkdirTemp("/tmp", "TestCallRecursive-*") - assert.False(t.T(), len(rootdir) == 0 || err != nil, "failed to create temp-directory for test. err = %v", err) - defer os.RemoveAll(rootdir) - for i := 0; i < 10000; i++ { - file1, err := os.CreateTemp(rootdir, "*") - assert.False(t.T(), file1 == nil || err != nil, "failed to create file in \"%s\" for test. err = %v", rootdir, err) - } - dirPath := rootdir - for i := 0; i < 100; i++ { - dirPath = path.Join(dirPath, "dir") - err = os.Mkdir(dirPath, 0755) - assert.False(t.T(), err != nil, "failed to create sub-directory \"%s\" in \"%s\". err = %v", dirPath, rootdir, err) - - file, err := os.CreateTemp(dirPath, "*****") - assert.False(t.T(), file == nil || err != nil, "failed to create file in \"%s\" for test. err = %v", dirPath, err) - } - - // Call the target utility. - err = callListRecursive(rootdir) - - // Test that it does not return error. - assert.Nil(t.T(), err) -} - -func (t *MainTest) TestCallListRecursiveOnUnpopulatedDirectory() { - // Set up a mini file-system to test on. - rootdir, err := os.MkdirTemp("/tmp", "TestCallRecursive-*") - assert.False(t.T(), len(rootdir) == 0 || err != nil, "failed to create temp-directory for test. err = %v", err) + rootdir, _ := os.MkdirTemp("/tmp", "TestCallListRecursive-*") + os.CreateTemp(rootdir, "abc-*.txt") defer os.RemoveAll(rootdir) // Call the target utility. - err = callListRecursive(rootdir) + err := callListRecursive(rootdir) // Test that it does not return error. assert.Nil(t.T(), err) From d380e47633e661c368da7a7b32745e5da8f52413 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Wed, 22 May 2024 13:27:28 +0000 Subject: [PATCH 26/36] add linter-error abt missing error-check --- main_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/main_test.go b/main_test.go index 6ca9c9c389..eedab0fb0b 100644 --- a/main_test.go +++ b/main_test.go @@ -216,11 +216,12 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal func (t *MainTest) TestCallListRecursiveOnExistingDirectory() { // Set up a mini file-system to test on. rootdir, _ := os.MkdirTemp("/tmp", "TestCallListRecursive-*") - os.CreateTemp(rootdir, "abc-*.txt") + _, err := os.CreateTemp(rootdir, "abc-*.txt") + assert.Nil(t.T(), err) defer os.RemoveAll(rootdir) // Call the target utility. - err := callListRecursive(rootdir) + err = callListRecursive(rootdir) // Test that it does not return error. assert.Nil(t.T(), err) From 9a81f6f641cabfc84f501b05200816a649eb6bb7 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 04:06:31 +0000 Subject: [PATCH 27/36] Rename metadata-prefetch-mode .. to metadata-prefetch-on-mount --- flags.go | 2 +- flags_test.go | 2 +- tools/integration_tests/operations/operations_test.go | 4 ++-- tools/mount_gcsfuse/main.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/flags.go b/flags.go index 5330e66075..37ab5b9998 100644 --- a/flags.go +++ b/flags.go @@ -37,7 +37,7 @@ const ( // MetadataPrefetchModeFlag is the name of the commandline flag for enabling // metadata-prefetch mode aka 'ls -R' during mount. - MetadataPrefetchModeFlag = "metadata-prefetch-mode" + MetadataPrefetchModeFlag = "metadata-prefetch-on-mount" ) // Set up custom help text for gcsfuse; in particular the usage section. diff --git a/flags_test.go b/flags_test.go index dd388bd172..a3fcb3f6c5 100644 --- a/flags_test.go +++ b/flags_test.go @@ -215,7 +215,7 @@ func (t *FlagsTest) Strings() { "--temp-dir=foobar", "--only-dir=baz", "--client-protocol=HTTP2", - "--metadata-prefetch-mode=asynchronous", + "--metadata-prefetch-on-mount=asynchronous", } f := parseArgs(t, args) diff --git a/tools/integration_tests/operations/operations_test.go b/tools/integration_tests/operations/operations_test.go index e868ebafd0..b530658ae6 100644 --- a/tools/integration_tests/operations/operations_test.go +++ b/tools/integration_tests/operations/operations_test.go @@ -132,7 +132,7 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { filePath3 := setup.YAMLConfigFile(mountConfig3, "config3.yaml") flags = append(flags, []string{"--config-file=" + filePath3}) - // Set up config file testing metadata-prefetch-mode: "asynchronous". + // Set up config file testing metadata-prefetch-on-mount: "asynchronous". mountConfig4 := config.MountConfig{ LogConfig: config.LogConfig{ Severity: config.TRACE, @@ -140,7 +140,7 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { }, } filePath4 := setup.YAMLConfigFile(mountConfig4, "config4.yaml") - flags = append(flags, []string{"--config-file=" + filePath4, "--metadata-prefetch-mode=asynchronous"}) + flags = append(flags, []string{"--config-file=" + filePath4, "--metadata-prefetch-on-mount=asynchronous"}) return flags } diff --git a/tools/mount_gcsfuse/main.go b/tools/mount_gcsfuse/main.go index 80f0c07a9e..63473832e4 100644 --- a/tools/mount_gcsfuse/main.go +++ b/tools/mount_gcsfuse/main.go @@ -122,7 +122,7 @@ func makeGcsfuseArgs( "log_file", "custom_endpoint", "config_file", - "metadata_prefetch_mode": + "metadata_prefetch_on_mount": args = append(args, "--"+strings.Replace(name, "_", "-", -1), value) // Special case: support mount-like formatting for gcsfuse debug flags. From 282e962894f5c4df62aceab4b4d39fd39e4bc367 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 04:12:52 +0000 Subject: [PATCH 28/36] Change [a]synchronous to [a]sync for md-prefetch Change "synchronous" to "sync", and "asynchronous" to "async" for values for metadata-prefetch-on-mount everywhere. --- flags.go | 2 +- flags_test.go | 4 ++-- internal/config/mount_config.go | 4 ++-- internal/config/yaml_parser.go | 2 +- tools/integration_tests/operations/operations_test.go | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/flags.go b/flags.go index 37ab5b9998..d4e42c4767 100644 --- a/flags.go +++ b/flags.go @@ -372,7 +372,7 @@ func newApp() (app *cli.App) { cli.StringFlag{ Name: MetadataPrefetchModeFlag, Value: config.DefaultMetadataPrefetchMode, - Usage: "This indicates whether or not to prefetch the metadata (prefilling of metadata caches and creation of inodes) of the mounted bucket at the time of mounting the bucket. Supported values: \"disabled\", \"synchronous\" and \"asynchronous\". Any other values will return error on mounting. This is applicable only to single-bucket mount-points, and not to dynamic-mount points.", + Usage: "This indicates whether or not to prefetch the metadata (prefilling of metadata caches and creation of inodes) of the mounted bucket at the time of mounting the bucket. Supported values: \"disabled\", \"sync\" and \"async\". Any other values will return error on mounting. This is applicable only to single-bucket mount-points, and not to dynamic-mount points.", }, }, } diff --git a/flags_test.go b/flags_test.go index a3fcb3f6c5..cb937aa126 100644 --- a/flags_test.go +++ b/flags_test.go @@ -215,7 +215,7 @@ func (t *FlagsTest) Strings() { "--temp-dir=foobar", "--only-dir=baz", "--client-protocol=HTTP2", - "--metadata-prefetch-on-mount=asynchronous", + "--metadata-prefetch-on-mount=async", } f := parseArgs(t, args) @@ -381,7 +381,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientPro func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { for _, input := range []string{ - "disabled", "synchronous", "asynchronous", + "disabled", "sync", "async", } { flags := &flagStorage{ // Unrelated fields, not being tested here, so set to sane values. diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 54a6e39201..c3c9f46ce0 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -54,9 +54,9 @@ const ( // MetadataPrefetchModeDisabled is the mode without metadata-prefetch. MetadataPrefetchModeDisabled string = "disabled" // MetadataPrefetchModeSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. - MetadataPrefetchModeSynchronous string = "synchronous" + MetadataPrefetchModeSynchronous string = "sync" // MetadataPrefetchModeAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. - MetadataPrefetchModeAsynchronous string = "asynchronous" + MetadataPrefetchModeAsynchronous string = "async" // DefaultMetadataPrefetchMode is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchModeDisabled. DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled ) diff --git a/internal/config/yaml_parser.go b/internal/config/yaml_parser.go index 87dd73f25a..4731b74cda 100644 --- a/internal/config/yaml_parser.go +++ b/internal/config/yaml_parser.go @@ -43,7 +43,7 @@ const ( StatCacheMaxSizeMBInvalidValueError = "the value of stat-cache-max-size-mb for metadata-cache can't be less than -1" StatCacheMaxSizeMBTooHighError = "the value of stat-cache-max-size-mb for metadata-cache is too high! Max supported: 17592186044415" MaxSupportedStatCacheMaxSizeMB = util.MaxMiBsInUint64 - UnsupportedMetadataPrefixModeError = "unsupported metadata-prefix-mode: \"%s\"; supported values: disabled, synchronous, asynchronous" + UnsupportedMetadataPrefixModeError = "unsupported metadata-prefix-mode: \"%s\"; supported values: disabled, sync, async" ) func IsValidLogSeverity(severity LogSeverity) bool { diff --git a/tools/integration_tests/operations/operations_test.go b/tools/integration_tests/operations/operations_test.go index b530658ae6..e61c5e78cf 100644 --- a/tools/integration_tests/operations/operations_test.go +++ b/tools/integration_tests/operations/operations_test.go @@ -132,7 +132,7 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { filePath3 := setup.YAMLConfigFile(mountConfig3, "config3.yaml") flags = append(flags, []string{"--config-file=" + filePath3}) - // Set up config file testing metadata-prefetch-on-mount: "asynchronous". + // Set up config file testing metadata-prefetch-on-mount: "async". mountConfig4 := config.MountConfig{ LogConfig: config.LogConfig{ Severity: config.TRACE, @@ -140,7 +140,7 @@ func createMountConfigsAndEquivalentFlags() (flags [][]string) { }, } filePath4 := setup.YAMLConfigFile(mountConfig4, "config4.yaml") - flags = append(flags, []string{"--config-file=" + filePath4, "--metadata-prefetch-on-mount=asynchronous"}) + flags = append(flags, []string{"--config-file=" + filePath4, "--metadata-prefetch-on-mount=async"}) return flags } From b30cee0e85e3e593c0b20afebf73d971cd49d814 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 04:52:07 +0000 Subject: [PATCH 29/36] address review comments --- main.go | 104 ++++++++++++++++++++++++--------------------------- main_test.go | 15 ++++---- 2 files changed, 56 insertions(+), 63 deletions(-) diff --git a/main.go b/main.go index 809b7f0c05..ec0f325e2d 100644 --- a/main.go +++ b/main.go @@ -391,33 +391,29 @@ func runCLIApp(c *cli.Context) (err error) { // Run. err = daemonize.Run(path, args, env, os.Stdout) if err != nil { - err = fmt.Errorf("daemonize.Run: %w", err) - return - } else { - if isDynamicMount { - logger.Infof(SuccessfulMountMessage) - } else { - switch flags.MetadataPrefetchMode { - case config.MetadataPrefetchModeDisabled: - logger.Infof(SuccessfulMountMessage) - case config.MetadataPrefetchModeSynchronous: - if err = callListRecursive(mountPoint); err != nil { - err = fmt.Errorf("metadata-prefetch failed: %w", err) - return - } - logger.Infof(SuccessfulMountMessage) - case config.MetadataPrefetchModeAsynchronous: - logger.Infof(SuccessfulMountMessage) - go func() { - if err := callListRecursive(mountPoint); err != nil { - logger.Errorf("metadata-prefetch failed: %v", err) - } - }() - } + return fmt.Errorf("daemonize.Run: %w", err) + } + if isDynamicMount { + logger.Infof(SuccessfulMountMessage) + return err + } + switch flags.MetadataPrefetchMode { + case config.MetadataPrefetchModeDisabled: + logger.Infof(SuccessfulMountMessage) + case config.MetadataPrefetchModeSynchronous: + if err = callListRecursive(mountPoint); err != nil { + return fmt.Errorf("metadata-prefetch failed: %w", err) } + logger.Infof(SuccessfulMountMessage) + case config.MetadataPrefetchModeAsynchronous: + logger.Infof(SuccessfulMountMessage) + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("metadata-prefetch failed: %v", err) + } + }() } - - return + return err } // The returned error is ignored as we do not enforce monitoring exporters @@ -445,43 +441,39 @@ func runCLIApp(c *cli.Context) (err error) { callDaemonizeSignalOutcome(nil) } - if err == nil { - if isDynamicMount { - markSuccessfulMount() - } else { - switch flags.MetadataPrefetchMode { - case config.MetadataPrefetchModeDisabled: - markSuccessfulMount() - case config.MetadataPrefetchModeSynchronous: - if err = callListRecursive(mountPoint); err != nil { - // Printing via mountStatus will have duplicate logs on the console while - // mounting gcsfuse in foreground mode. But this is important to avoid - // losing error logs when run in the background mode. - logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) - err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) - callDaemonizeSignalOutcome(err) - return - } - - markSuccessfulMount() - case config.MetadataPrefetchModeAsynchronous: - markSuccessfulMount() - - go func() { - if err := callListRecursive(mountPoint); err != nil { - logger.Errorf("Metadata-prefetch failed: %v", err) - } - }() - } - } - } else { + markFailure := func(err error) { // Printing via mountStatus will have duplicate logs on the console while // mounting gcsfuse in foreground mode. But this is important to avoid // losing error logs when run in the background mode. logger.Errorf("%s: %v\n", UnsuccessfulMountMessagePrefix, err) err = fmt.Errorf("%s: mountWithArgs: %w", UnsuccessfulMountMessagePrefix, err) callDaemonizeSignalOutcome(err) - return + } + + if err != nil { + markFailure(err) + return err + } + if isDynamicMount { + markSuccessfulMount() + } else { + switch flags.MetadataPrefetchMode { + case config.MetadataPrefetchModeDisabled: + markSuccessfulMount() + case config.MetadataPrefetchModeSynchronous: + if err = callListRecursive(mountPoint); err != nil { + markFailure(err) + return err + } + markSuccessfulMount() + case config.MetadataPrefetchModeAsynchronous: + markSuccessfulMount() + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("Metadata-prefetch failed: %v", err) + } + }() + } } } diff --git a/main_test.go b/main_test.go index eedab0fb0b..53999fc4ca 100644 --- a/main_test.go +++ b/main_test.go @@ -215,15 +215,18 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal func (t *MainTest) TestCallListRecursiveOnExistingDirectory() { // Set up a mini file-system to test on. - rootdir, _ := os.MkdirTemp("/tmp", "TestCallListRecursive-*") - _, err := os.CreateTemp(rootdir, "abc-*.txt") - assert.Nil(t.T(), err) + rootdir, err := os.MkdirTemp("/tmp", "TestCallListRecursive-*") + if err != nil { + t.T().Fatalf("Failed to set up test. error = %v", err) + } defer os.RemoveAll(rootdir) + _, err = os.CreateTemp(rootdir, "abc-*.txt") + if err != nil { + t.T().Fatalf("Failed to set up test. error = %v", err) + } - // Call the target utility. err = callListRecursive(rootdir) - // Test that it does not return error. assert.Nil(t.T(), err) } @@ -231,9 +234,7 @@ func (t *MainTest) TestCallListRecursiveOnNonExistingDirectory() { // Set up a mini file-system to test on, which must fail. rootdir := "/path/to/non/existing/directory" - // Call the target utility. err := callListRecursive(rootdir) - // Test that it returns error. assert.ErrorContains(t.T(), err, "does not exist") } From d4eb6d229300f3044b1f123c9c857fc12afe8222 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 04:59:57 +0000 Subject: [PATCH 30/36] MetadataPrefetchMode -> MetadataPrefetchOnMount Changes in all internal state variables/ constants etc. needed to fix the mount-flags log. --- flags.go | 28 +++++++++++----------- flags_test.go | 42 ++++++++++++++++----------------- internal/config/mount_config.go | 16 ++++++------- main.go | 16 ++++++------- main_test.go | 2 +- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/flags.go b/flags.go index d4e42c4767..ecafb11d27 100644 --- a/flags.go +++ b/flags.go @@ -35,9 +35,9 @@ const ( // maxSequentialReadSizeMb is the max value supported by sequential-read-size-mb flag. maxSequentialReadSizeMb = 1024 - // MetadataPrefetchModeFlag is the name of the commandline flag for enabling + // MetadataPrefetchOnMountFlag is the name of the commandline flag for enabling // metadata-prefetch mode aka 'ls -R' during mount. - MetadataPrefetchModeFlag = "metadata-prefetch-on-mount" + MetadataPrefetchOnMountFlag = "metadata-prefetch-on-mount" ) // Set up custom help text for gcsfuse; in particular the usage section. @@ -370,8 +370,8 @@ func newApp() (app *cli.App) { ///////////////////////// cli.StringFlag{ - Name: MetadataPrefetchModeFlag, - Value: config.DefaultMetadataPrefetchMode, + Name: MetadataPrefetchOnMountFlag, + Value: config.DefaultMetadataPrefetchOnMount, Usage: "This indicates whether or not to prefetch the metadata (prefilling of metadata caches and creation of inodes) of the mounted bucket at the time of mounting the bucket. Supported values: \"disabled\", \"sync\" and \"async\". Any other values will return error on mounting. This is applicable only to single-bucket mount-points, and not to dynamic-mount points.", }, }, @@ -440,12 +440,12 @@ type flagStorage struct { // Post-mount actions - // MetadataPrefetchMode indicates whether or not to prefetch the metadata of the mounted bucket at the time of mounting the bucket. - // Supported values: MetadataPrefetchModeDisabled, MetadataPrefetchModeSynchronous, and MetadataPrefetchModeAsynchronous. + // MetadataPrefetchOnMount indicates whether or not to prefetch the metadata of the mounted bucket at the time of mounting the bucket. + // Supported values: MetadataPrefetchOnMountDisabled, MetadataPrefetchOnMountSynchronous, and MetadataPrefetchOnMountAsynchronous. // Any other values will return error on mounting. // This is applicable only to single-bucket mount-points, and not to dynamic-mount points. This is because dynamic-mounts don't mount the bucket(s) at the time of // gcsfuse command itself, which flag is targeted at. - MetadataPrefetchMode string + MetadataPrefetchOnMount string } func resolveFilePath(filePath string, configKey string) (resolvedPath string, err error) { @@ -588,7 +588,7 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) { DebugMutex: c.Bool("debug_mutex"), // Post-mount actions - MetadataPrefetchMode: c.String(MetadataPrefetchModeFlag), + MetadataPrefetchOnMount: c.String(MetadataPrefetchOnMountFlag), } // Handle the repeated "-o" flag. @@ -601,13 +601,13 @@ func populateFlags(c *cli.Context) (flags *flagStorage, err error) { return } -func validateMetadataPrefetchMode(mode string) error { +func validateMetadataPrefetchOnMount(mode string) error { switch mode { - case config.MetadataPrefetchModeDisabled: + case config.MetadataPrefetchOnMountDisabled: fallthrough - case config.MetadataPrefetchModeSynchronous: + case config.MetadataPrefetchOnMountSynchronous: fallthrough - case config.MetadataPrefetchModeAsynchronous: + case config.MetadataPrefetchOnMountAsynchronous: return nil default: return fmt.Errorf(config.UnsupportedMetadataPrefixModeError, mode) @@ -623,8 +623,8 @@ func validateFlags(flags *flagStorage) (err error) { return fmt.Errorf("client protocol: %s is not valid", flags.ClientProtocol) } - if err = validateMetadataPrefetchMode(flags.MetadataPrefetchMode); err != nil { - return fmt.Errorf("%s: is not valid; error = %w", MetadataPrefetchModeFlag, err) + if err = validateMetadataPrefetchOnMount(flags.MetadataPrefetchOnMount); err != nil { + return fmt.Errorf("%s: is not valid; error = %w", MetadataPrefetchOnMountFlag, err) } return nil diff --git a/flags_test.go b/flags_test.go index cb937aa126..0c3c83b608 100644 --- a/flags_test.go +++ b/flags_test.go @@ -104,7 +104,7 @@ func (t *FlagsTest) Defaults() { assert.False(t.T(), f.DebugInvariants) // Post-mount actions - assert.Equal(t.T(), config.MetadataPrefetchModeDisabled, f.MetadataPrefetchMode) + assert.Equal(t.T(), config.MetadataPrefetchOnMountDisabled, f.MetadataPrefetchOnMount) } func (t *FlagsTest) Bools() { @@ -223,7 +223,7 @@ func (t *FlagsTest) Strings() { assert.Equal(t.T(), "foobar", f.TempDir) assert.Equal(t.T(), "baz", f.OnlyDir) assert.Equal(t.T(), mountpkg.HTTP2, f.ClientProtocol) - assert.Equal(t.T(), config.MetadataPrefetchModeAsynchronous, f.MetadataPrefetchMode) + assert.Equal(t.T(), config.MetadataPrefetchOnMountAsynchronous, f.MetadataPrefetchOnMount) } func (t *FlagsTest) Durations() { @@ -319,9 +319,9 @@ func (t *FlagsTest) TestResolvePathForTheFlagsInContext() { func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP1ClientProtocol() { flags := &flagStorage{ - SequentialReadSizeMb: 10, - ClientProtocol: mountpkg.ClientProtocol("http1"), - MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, + SequentialReadSizeMb: 10, + ClientProtocol: mountpkg.ClientProtocol("http1"), + MetadataPrefetchOnMount: config.DefaultMetadataPrefetchOnMount, } err := validateFlags(flags) @@ -331,9 +331,9 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP1ClientPro func (t *FlagsTest) TestValidateFlagsForZeroSequentialReadSizeAndValidClientProtocol() { flags := &flagStorage{ - SequentialReadSizeMb: 0, - ClientProtocol: mountpkg.ClientProtocol("http2"), - MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, + SequentialReadSizeMb: 0, + ClientProtocol: mountpkg.ClientProtocol("http2"), + MetadataPrefetchOnMount: config.DefaultMetadataPrefetchOnMount, } err := validateFlags(flags) @@ -344,9 +344,9 @@ func (t *FlagsTest) TestValidateFlagsForZeroSequentialReadSizeAndValidClientProt func (t *FlagsTest) TestValidateFlagsForSequentialReadSizeGreaterThan1024AndValidClientProtocol() { flags := &flagStorage{ - SequentialReadSizeMb: 2048, - ClientProtocol: mountpkg.ClientProtocol("http1"), - MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, + SequentialReadSizeMb: 2048, + ClientProtocol: mountpkg.ClientProtocol("http1"), + MetadataPrefetchOnMount: config.DefaultMetadataPrefetchOnMount, } err := validateFlags(flags) @@ -357,9 +357,9 @@ func (t *FlagsTest) TestValidateFlagsForSequentialReadSizeGreaterThan1024AndVali func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndInValidClientProtocol() { flags := &flagStorage{ - SequentialReadSizeMb: 10, - ClientProtocol: mountpkg.ClientProtocol("http4"), - MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, + SequentialReadSizeMb: 10, + ClientProtocol: mountpkg.ClientProtocol("http4"), + MetadataPrefetchOnMount: config.DefaultMetadataPrefetchOnMount, } err := validateFlags(flags) @@ -369,9 +369,9 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndInValidClientP func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientProtocol() { flags := &flagStorage{ - SequentialReadSizeMb: 10, - ClientProtocol: mountpkg.ClientProtocol("http2"), - MetadataPrefetchMode: config.DefaultMetadataPrefetchMode, + SequentialReadSizeMb: 10, + ClientProtocol: mountpkg.ClientProtocol("http2"), + MetadataPrefetchOnMount: config.DefaultMetadataPrefetchOnMount, } err := validateFlags(flags) @@ -379,7 +379,7 @@ func (t *FlagsTest) TestValidateFlagsForValidSequentialReadSizeAndHTTP2ClientPro assert.Equal(t.T(), nil, err) } -func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { +func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchOnMount() { for _, input := range []string{ "disabled", "sync", "async", } { @@ -388,7 +388,7 @@ func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { SequentialReadSizeMb: 200, ClientProtocol: mountpkg.ClientProtocol("http2"), // The flag being tested. - MetadataPrefetchMode: input, + MetadataPrefetchOnMount: input, } err := validateFlags(flags) @@ -397,7 +397,7 @@ func (t *FlagsTest) TestValidateFlagsForSupportedMetadataPrefetchMode() { } } -func (t *FlagsTest) TestValidateFlagsForUnsupportedMetadataPrefetchMode() { +func (t *FlagsTest) TestValidateFlagsForUnsupportedMetadataPrefetchOnMount() { for _, input := range []string{ "", "unsupported", } { @@ -406,7 +406,7 @@ func (t *FlagsTest) TestValidateFlagsForUnsupportedMetadataPrefetchMode() { SequentialReadSizeMb: 200, ClientProtocol: mountpkg.ClientProtocol("http2"), // The flag being tested. - MetadataPrefetchMode: input, + MetadataPrefetchOnMount: input, } err := validateFlags(flags) diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index c3c9f46ce0..f5acf7a780 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -51,14 +51,14 @@ const ( DefaultAnonymousAccess = false DefaultEnableHNS = false - // MetadataPrefetchModeDisabled is the mode without metadata-prefetch. - MetadataPrefetchModeDisabled string = "disabled" - // MetadataPrefetchModeSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. - MetadataPrefetchModeSynchronous string = "sync" - // MetadataPrefetchModeAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. - MetadataPrefetchModeAsynchronous string = "async" - // DefaultMetadataPrefetchMode is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchModeDisabled. - DefaultMetadataPrefetchMode = MetadataPrefetchModeDisabled + // MetadataPrefetchOnMountDisabled is the mode without metadata-prefetch. + MetadataPrefetchOnMountDisabled string = "disabled" + // MetadataPrefetchOnMountSynchronous is the prefetch-mode where mounting is not marked complete until prefetch is complete. + MetadataPrefetchOnMountSynchronous string = "sync" + // MetadataPrefetchOnMountAsynchronous is the prefetch-mode where mounting is marked complete once prefetch has started. + MetadataPrefetchOnMountAsynchronous string = "async" + // DefaultMetadataPrefetchOnMount is default value of metadata-prefetch i.e. if not set by user; current it is MetadataPrefetchOnMountDisabled. + DefaultMetadataPrefetchOnMount = MetadataPrefetchOnMountDisabled ) type WriteConfig struct { diff --git a/main.go b/main.go index ec0f325e2d..35cb901cd9 100644 --- a/main.go +++ b/main.go @@ -397,15 +397,15 @@ func runCLIApp(c *cli.Context) (err error) { logger.Infof(SuccessfulMountMessage) return err } - switch flags.MetadataPrefetchMode { - case config.MetadataPrefetchModeDisabled: + switch flags.MetadataPrefetchOnMount { + case config.MetadataPrefetchOnMountDisabled: logger.Infof(SuccessfulMountMessage) - case config.MetadataPrefetchModeSynchronous: + case config.MetadataPrefetchOnMountSynchronous: if err = callListRecursive(mountPoint); err != nil { return fmt.Errorf("metadata-prefetch failed: %w", err) } logger.Infof(SuccessfulMountMessage) - case config.MetadataPrefetchModeAsynchronous: + case config.MetadataPrefetchOnMountAsynchronous: logger.Infof(SuccessfulMountMessage) go func() { if err := callListRecursive(mountPoint); err != nil { @@ -457,16 +457,16 @@ func runCLIApp(c *cli.Context) (err error) { if isDynamicMount { markSuccessfulMount() } else { - switch flags.MetadataPrefetchMode { - case config.MetadataPrefetchModeDisabled: + switch flags.MetadataPrefetchOnMount { + case config.MetadataPrefetchOnMountDisabled: markSuccessfulMount() - case config.MetadataPrefetchModeSynchronous: + case config.MetadataPrefetchOnMountSynchronous: if err = callListRecursive(mountPoint); err != nil { markFailure(err) return err } markSuccessfulMount() - case config.MetadataPrefetchModeAsynchronous: + case config.MetadataPrefetchOnMountAsynchronous: markSuccessfulMount() go func() { if err := callListRecursive(mountPoint); err != nil { diff --git a/main_test.go b/main_test.go index 53999fc4ca..3ddfba79f6 100644 --- a/main_test.go +++ b/main_test.go @@ -209,7 +209,7 @@ func (t *MainTest) TestStringifyShouldReturnAllFlagsPassedInFlagStorageAsMarshal actual, err := util.Stringify(flags) assert.Equal(t.T(), nil, err) - expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false,\"MetadataPrefetchMode\":\"\"}" + expected := "{\"AppName\":\"\",\"Foreground\":false,\"ConfigFile\":\"\",\"MountOptions\":{\"1\":\"one\",\"2\":\"two\",\"3\":\"three\"},\"DirMode\":0,\"FileMode\":0,\"Uid\":0,\"Gid\":0,\"ImplicitDirs\":false,\"OnlyDir\":\"\",\"RenameDirLimit\":0,\"IgnoreInterrupts\":false,\"CustomEndpoint\":null,\"BillingProject\":\"\",\"KeyFile\":\"\",\"TokenUrl\":\"\",\"ReuseTokenFromUrl\":false,\"EgressBandwidthLimitBytesPerSecond\":0,\"OpRateLimitHz\":0,\"SequentialReadSizeMb\":10,\"AnonymousAccess\":false,\"MaxRetrySleep\":0,\"StatCacheCapacity\":0,\"StatCacheTTL\":0,\"TypeCacheTTL\":0,\"HttpClientTimeout\":0,\"MaxRetryDuration\":0,\"RetryMultiplier\":0,\"LocalFileCache\":false,\"TempDir\":\"\",\"ClientProtocol\":\"http4\",\"MaxConnsPerHost\":0,\"MaxIdleConnsPerHost\":0,\"EnableNonexistentTypeCache\":false,\"StackdriverExportInterval\":0,\"OtelCollectorAddress\":\"\",\"LogFile\":\"\",\"LogFormat\":\"\",\"ExperimentalEnableJsonRead\":false,\"DebugFuseErrors\":false,\"DebugFuse\":false,\"DebugFS\":false,\"DebugGCS\":false,\"DebugHTTP\":false,\"DebugInvariants\":false,\"DebugMutex\":false,\"MetadataPrefetchOnMount\":\"\"}" assert.Equal(t.T(), expected, actual) } From bd85941d03c875d8734cf6b356645424b2afbca0 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 08:16:23 +0000 Subject: [PATCH 31/36] Optimize code This creates a mini-race condition between the debug log for successful and that of starting the recursive listing of mountPoint when metadata-prefetch-on-mount has been set to "async", but it is immaterial to the end user. --- main.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/main.go b/main.go index 35cb901cd9..ea55f650ab 100644 --- a/main.go +++ b/main.go @@ -214,16 +214,14 @@ func callListRecursive(mountPoint string) (err error) { logger.Debugf("Started recursive metadata-prefetch of directory: \"%s\" ...", mountPoint) numItems := 0 err = filepath.WalkDir(mountPoint, func(path string, d fs.DirEntry, err error) error { - if err != nil { - if d == nil { - return fmt.Errorf("got error walking: path=\"%s\" does not exist, error = %w", path, err) - } else { - return fmt.Errorf("got error walking: path=\"%s\", dentry=\"%s\", isDir=%v, error = %w", path, d.Name(), d.IsDir(), err) - } + if err == nil { + numItems++ + return err } - - numItems++ - return err + if d == nil { + return fmt.Errorf("got error walking: path=\"%s\" does not exist, error = %w", path, err) + } + return fmt.Errorf("got error walking: path=\"%s\", dentry=\"%s\", isDir=%v, error = %w", path, d.Name(), d.IsDir(), err) }) if err != nil { @@ -398,21 +396,18 @@ func runCLIApp(c *cli.Context) (err error) { return err } switch flags.MetadataPrefetchOnMount { - case config.MetadataPrefetchOnMountDisabled: - logger.Infof(SuccessfulMountMessage) case config.MetadataPrefetchOnMountSynchronous: if err = callListRecursive(mountPoint); err != nil { return fmt.Errorf("metadata-prefetch failed: %w", err) } - logger.Infof(SuccessfulMountMessage) case config.MetadataPrefetchOnMountAsynchronous: - logger.Infof(SuccessfulMountMessage) go func() { if err := callListRecursive(mountPoint); err != nil { logger.Errorf("metadata-prefetch failed: %v", err) } }() } + logger.Infof(SuccessfulMountMessage) return err } @@ -441,7 +436,7 @@ func runCLIApp(c *cli.Context) (err error) { callDaemonizeSignalOutcome(nil) } - markFailure := func(err error) { + markMountFailure := func(err error) { // Printing via mountStatus will have duplicate logs on the console while // mounting gcsfuse in foreground mode. But this is important to avoid // losing error logs when run in the background mode. @@ -451,29 +446,26 @@ func runCLIApp(c *cli.Context) (err error) { } if err != nil { - markFailure(err) + markMountFailure(err) return err } if isDynamicMount { markSuccessfulMount() } else { switch flags.MetadataPrefetchOnMount { - case config.MetadataPrefetchOnMountDisabled: - markSuccessfulMount() case config.MetadataPrefetchOnMountSynchronous: if err = callListRecursive(mountPoint); err != nil { - markFailure(err) + markMountFailure(err) return err } - markSuccessfulMount() case config.MetadataPrefetchOnMountAsynchronous: - markSuccessfulMount() go func() { if err := callListRecursive(mountPoint); err != nil { logger.Errorf("Metadata-prefetch failed: %v", err) } }() } + markSuccessfulMount() } } From e3c27fed1f719839d03a1a5834888048711b137b Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 09:46:56 +0000 Subject: [PATCH 32/36] handle _ as bucket for dynamic-mount --- main.go | 10 ++++++---- main_test.go | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index ea55f650ab..6f99e26258 100644 --- a/main.go +++ b/main.go @@ -233,6 +233,10 @@ func callListRecursive(mountPoint string) (err error) { return nil } +func isDynamicMount(bucketName string) bool { + return bucketName == "" || bucketName == "_" +} + func runCLIApp(c *cli.Context) (err error) { err = resolvePathForTheFlagsInContext(c) if err != nil { @@ -279,8 +283,6 @@ func runCLIApp(c *cli.Context) (err error) { return } - isDynamicMount := bucketName == "" - logger.Infof("Start gcsfuse/%s for app %q using mount point: %s\n", getVersion(), flags.AppName, mountPoint) // Log mount-config and the CLI flags in the log-file. @@ -391,7 +393,7 @@ func runCLIApp(c *cli.Context) (err error) { if err != nil { return fmt.Errorf("daemonize.Run: %w", err) } - if isDynamicMount { + if isDynamicMount(bucketName) { logger.Infof(SuccessfulMountMessage) return err } @@ -449,7 +451,7 @@ func runCLIApp(c *cli.Context) (err error) { markMountFailure(err) return err } - if isDynamicMount { + if isDynamicMount(bucketName) { markSuccessfulMount() } else { switch flags.MetadataPrefetchOnMount { diff --git a/main_test.go b/main_test.go index 3ddfba79f6..5690b11b91 100644 --- a/main_test.go +++ b/main_test.go @@ -238,3 +238,23 @@ func (t *MainTest) TestCallListRecursiveOnNonExistingDirectory() { assert.ErrorContains(t.T(), err, "does not exist") } + +func (t *MainTest) TestIsDynamicMount() { + for _, input := range []struct { + bucketName string + isDynamic bool + }{ + { + bucketName: "", + isDynamic: true, + }, { + bucketName: "_", + isDynamic: true, + }, { + bucketName: "abc", + isDynamic: false, + }, + } { + assert.Equal(t.T(), input.isDynamic, isDynamicMount(input.bucketName)) + } +} From a54ef3e0f7609472991ea634fd9be21e6b8124be Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 10:04:05 +0000 Subject: [PATCH 33/36] optimize code --- main.go | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/main.go b/main.go index 6f99e26258..a72d715196 100644 --- a/main.go +++ b/main.go @@ -393,21 +393,19 @@ func runCLIApp(c *cli.Context) (err error) { if err != nil { return fmt.Errorf("daemonize.Run: %w", err) } - if isDynamicMount(bucketName) { - logger.Infof(SuccessfulMountMessage) - return err - } - switch flags.MetadataPrefetchOnMount { - case config.MetadataPrefetchOnMountSynchronous: - if err = callListRecursive(mountPoint); err != nil { - return fmt.Errorf("metadata-prefetch failed: %w", err) - } - case config.MetadataPrefetchOnMountAsynchronous: - go func() { - if err := callListRecursive(mountPoint); err != nil { - logger.Errorf("metadata-prefetch failed: %v", err) + if !isDynamicMount(bucketName) { + switch flags.MetadataPrefetchOnMount { + case config.MetadataPrefetchOnMountSynchronous: + if err = callListRecursive(mountPoint); err != nil { + return fmt.Errorf("metadata-prefetch failed: %w", err) } - }() + case config.MetadataPrefetchOnMountAsynchronous: + go func() { + if err := callListRecursive(mountPoint); err != nil { + logger.Errorf("metadata-prefetch failed: %v", err) + } + }() + } } logger.Infof(SuccessfulMountMessage) return err @@ -451,9 +449,7 @@ func runCLIApp(c *cli.Context) (err error) { markMountFailure(err) return err } - if isDynamicMount(bucketName) { - markSuccessfulMount() - } else { + if !isDynamicMount(bucketName) { switch flags.MetadataPrefetchOnMount { case config.MetadataPrefetchOnMountSynchronous: if err = callListRecursive(mountPoint); err != nil { @@ -467,8 +463,8 @@ func runCLIApp(c *cli.Context) (err error) { } }() } - markSuccessfulMount() } + markSuccessfulMount() } // Let the user unmount with Ctrl-C (SIGINT). From 74b2d37045ffafdf27ec4667e56f3b3475818c7f Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 10:18:05 +0000 Subject: [PATCH 34/36] improve error logs --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index a72d715196..def0c6d35a 100644 --- a/main.go +++ b/main.go @@ -426,7 +426,7 @@ func runCLIApp(c *cli.Context) (err error) { // logging them as error logs. callDaemonizeSignalOutcome := func(err error) { if err2 := daemonize.SignalOutcome(err); err2 != nil { - logger.Errorf("Failed to signal to daemon: %v", err2) + logger.Errorf("Failed to signal error to parent-process from daemon: %v", err2) } } From 041f8811f60ccb5382cb2998dc6d25cdf47a6946 Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Thu, 23 May 2024 10:49:57 +0000 Subject: [PATCH 35/36] Remove callList from daemon-parent Removed redundant call to callRecursiveList from the parent process of the GCSFuse daemon. --- main.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/main.go b/main.go index def0c6d35a..d4e67bf934 100644 --- a/main.go +++ b/main.go @@ -393,20 +393,6 @@ func runCLIApp(c *cli.Context) (err error) { if err != nil { return fmt.Errorf("daemonize.Run: %w", err) } - if !isDynamicMount(bucketName) { - switch flags.MetadataPrefetchOnMount { - case config.MetadataPrefetchOnMountSynchronous: - if err = callListRecursive(mountPoint); err != nil { - return fmt.Errorf("metadata-prefetch failed: %w", err) - } - case config.MetadataPrefetchOnMountAsynchronous: - go func() { - if err := callListRecursive(mountPoint); err != nil { - logger.Errorf("metadata-prefetch failed: %v", err) - } - }() - } - } logger.Infof(SuccessfulMountMessage) return err } From e554469323c4eeb8c08afa85a7fe931129228ead Mon Sep 17 00:00:00 2001 From: Nitin Garg Date: Fri, 24 May 2024 06:59:22 +0000 Subject: [PATCH 36/36] Code optimization Reuse isDynamicMount utility in mount.go --- mount.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount.go b/mount.go index 112c104e96..23805c10dc 100644 --- a/mount.go +++ b/mount.go @@ -133,7 +133,7 @@ be interacting with the file system.`) } fsName := bucketName - if bucketName == "" || bucketName == "_" { + if isDynamicMount(bucketName) { // mounting all the buckets at once fsName = "gcsfuse" }