From 21f3b415c0ba81b07fad9c507d49153edfc82926 Mon Sep 17 00:00:00 2001 From: Ayush Sethi Date: Mon, 1 Apr 2024 14:14:33 +0000 Subject: [PATCH] Changes to allow parallel dirops and lookups --- go.mod | 2 +- go.sum | 6 ++++-- internal/config/mount_config.go | 4 ++-- .../invalid_disable_parallel_dirops.yaml | 2 ++ .../unset_disable_parallel_dirops.yaml | 2 ++ internal/config/testdata/valid_config.yaml | 1 + internal/config/yaml_parser_test.go | 17 +++++++++++++++++ internal/fs/fs.go | 9 +++++++-- internal/fs/inode/base_dir.go | 8 ++++++++ internal/fs/inode/dir.go | 19 +++++++++++++++---- main_test.go | 4 ++-- mount.go | 9 +++++---- 12 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 internal/config/testdata/file_system_config/invalid_disable_parallel_dirops.yaml create mode 100644 internal/config/testdata/file_system_config/unset_disable_parallel_dirops.yaml diff --git a/go.mod b/go.mod index 4106dabf77..c5b72c2608 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/google/uuid v1.6.0 github.com/googleapis/gax-go/v2 v2.12.3 github.com/jacobsa/daemonize v0.0.0-20160101105449-e460293e890f - github.com/jacobsa/fuse v0.0.0-20231003132804-d0f3daf365c3 + github.com/jacobsa/fuse v0.0.0-20240509083815-39f95ce809a8 github.com/jacobsa/oglematchers v0.0.0-20150720000706-141901ea67cd github.com/jacobsa/oglemock v0.0.0-20150831005832-e94d794d06ff github.com/jacobsa/ogletest v0.0.0-20170503003838-80d50a735a11 diff --git a/go.sum b/go.sum index 3a072c0fc2..10e2cb7b10 100644 --- a/go.sum +++ b/go.sum @@ -716,8 +716,10 @@ github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56/go.mod h1:ymszkNOg6t github.com/j-keck/arping v1.0.2/go.mod h1:aJbELhR92bSk7tp79AWM/ftfc90EfEi2bQJrbBFOsPw= github.com/jacobsa/daemonize v0.0.0-20160101105449-e460293e890f h1:X+tnaqoCcBgAwSTJtoYW6p0qKiuPyMfofEHEFUf2kdU= github.com/jacobsa/daemonize v0.0.0-20160101105449-e460293e890f/go.mod h1:Ip4fOwzCrnDVuluHBd7FXIMb7SHOKfkt9/UDrYSZvqI= -github.com/jacobsa/fuse v0.0.0-20231003132804-d0f3daf365c3 h1:NhlhxaD3w7/+ztfJzZHWJ9FeNPl3kDYQPXQ/arbXAbA= -github.com/jacobsa/fuse v0.0.0-20231003132804-d0f3daf365c3/go.mod h1:XUKuYy1M4vamyxQjW8/WZBTxyZ0NnUiq+kkA+WWOfeI= +github.com/jacobsa/fuse v0.0.0-20240419171848-edf18a690d4b h1:WelCHTd/WEv4E2XCARZwmQpKlVDxInTPLZROxVgRktI= +github.com/jacobsa/fuse v0.0.0-20240419171848-edf18a690d4b/go.mod h1:JYi9iIxdYNgxmMgLwtSHO/hmVnP2kfX1oc+mtx+XWLA= +github.com/jacobsa/fuse v0.0.0-20240509083815-39f95ce809a8 h1:5Meita5JExZswnw1muPoaKp1oDvSC4KbB9e8FBNTr8U= +github.com/jacobsa/fuse v0.0.0-20240509083815-39f95ce809a8/go.mod h1:JYi9iIxdYNgxmMgLwtSHO/hmVnP2kfX1oc+mtx+XWLA= github.com/jacobsa/oglematchers v0.0.0-20150720000706-141901ea67cd h1:9GCSedGjMcLZCrusBZuo4tyKLpKUPenUUqi34AkuFmA= github.com/jacobsa/oglematchers v0.0.0-20150720000706-141901ea67cd/go.mod h1:TlmyIZDpGmwRoTWiakdr+HA1Tukze6C6XbRVidYq02M= github.com/jacobsa/oglemock v0.0.0-20150831005832-e94d794d06ff h1:2xRHTvkpJ5zJmglXLRqHiZQNjUoOkhUyhTAhEQvPAWw= diff --git a/internal/config/mount_config.go b/internal/config/mount_config.go index 1326b9c509..715a6a100d 100644 --- a/internal/config/mount_config.go +++ b/internal/config/mount_config.go @@ -90,7 +90,8 @@ type EnableHNS bool type CacheDir string type FileSystemConfig struct { - IgnoreInterrupts bool `yaml:"ignore-interrupts"` + IgnoreInterrupts bool `yaml:"ignore-interrupts"` + DisableParallelDirops bool `yaml:"disable-parallel-dirops"` } type FileCacheConfig struct { @@ -180,6 +181,5 @@ func NewMountConfig() *MountConfig { AnonymousAccess: DefaultAnonymousAccess, } mountConfig.EnableHNS = DefaultEnableHNS - return mountConfig } diff --git a/internal/config/testdata/file_system_config/invalid_disable_parallel_dirops.yaml b/internal/config/testdata/file_system_config/invalid_disable_parallel_dirops.yaml new file mode 100644 index 0000000000..1109956504 --- /dev/null +++ b/internal/config/testdata/file_system_config/invalid_disable_parallel_dirops.yaml @@ -0,0 +1,2 @@ +file-system: + disable-parallel-dirops: -1 diff --git a/internal/config/testdata/file_system_config/unset_disable_parallel_dirops.yaml b/internal/config/testdata/file_system_config/unset_disable_parallel_dirops.yaml new file mode 100644 index 0000000000..4db0c34ced --- /dev/null +++ b/internal/config/testdata/file_system_config/unset_disable_parallel_dirops.yaml @@ -0,0 +1,2 @@ +write: + create-empty-file: true diff --git a/internal/config/testdata/valid_config.yaml b/internal/config/testdata/valid_config.yaml index d3525e9fbc..00c99b61a3 100644 --- a/internal/config/testdata/valid_config.yaml +++ b/internal/config/testdata/valid_config.yaml @@ -25,3 +25,4 @@ grpc: enable-hns: true file-system: ignore-interrupts: true + disable-parallel-dirops: true diff --git a/internal/config/yaml_parser_test.go b/internal/config/yaml_parser_test.go index 45ebab9236..0d73fd464f 100644 --- a/internal/config/yaml_parser_test.go +++ b/internal/config/yaml_parser_test.go @@ -47,6 +47,7 @@ func validateDefaultConfig(mountConfig *MountConfig) { ExpectEq(false, mountConfig.AuthConfig.AnonymousAccess) ExpectEq(false, mountConfig.EnableHNS) ExpectFalse(mountConfig.FileSystemConfig.IgnoreInterrupts) + ExpectEq(false, mountConfig.FileSystemConfig.DisableParallelDirops) } func (t *YamlParserTest) TestReadConfigFile_EmptyFileName() { @@ -130,6 +131,7 @@ func (t *YamlParserTest) TestReadConfigFile_ValidConfig() { // file-system config ExpectTrue(mountConfig.FileSystemConfig.IgnoreInterrupts) + ExpectTrue(mountConfig.FileSystemConfig.DisableParallelDirops) } func (t *YamlParserTest) TestReadConfigFile_InvalidLogConfig() { @@ -266,3 +268,18 @@ func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetAnonymousAcces AssertNe(nil, mountConfig) AssertEq(false, mountConfig.AuthConfig.AnonymousAccess) } + +func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_InvalidDisableParallelDirops() { + _, err := ParseConfigFile("testdata/file_system_config/invalid_disable_parallel_dirops.yaml") + + AssertNe(nil, err) + AssertTrue(strings.Contains(err.Error(), "error parsing config file: yaml: unmarshal errors:\n line 2: cannot unmarshal !!int `-1` into bool")) +} + +func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetDisableParallelDirops() { + mountConfig, err := ParseConfigFile("testdata/file_system_config/unset_disable_parallel_dirops.yaml") + + AssertEq(nil, err) + AssertNe(nil, mountConfig) + AssertEq(false, mountConfig.FileSystemConfig.DisableParallelDirops) +} diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 0724748392..88ccf8012e 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -924,8 +924,13 @@ func (fs *fileSystem) lookUpOrCreateChildInode( // Set up a function that will find a lookup result for the child with the // given name. Expects no locks to be held. getLookupResult := func() (*inode.Core, error) { - parent.Lock() - defer parent.Unlock() + if fs.mountConfig.FileSystemConfig.DisableParallelDirops { + parent.Lock() + defer parent.Unlock() + } else { + parent.LockForChildLookup() + defer parent.UnlockForChildLookup() + } return parent.LookUpChild(ctx, childName) } diff --git a/internal/fs/inode/base_dir.go b/internal/fs/inode/base_dir.go index 7478b5f571..06478fce66 100644 --- a/internal/fs/inode/base_dir.go +++ b/internal/fs/inode/base_dir.go @@ -92,6 +92,14 @@ func (d *baseDirInode) Unlock() { d.mu.Unlock() } +func (d *baseDirInode) LockForChildLookup() { + d.mu.Lock() +} + +func (d *baseDirInode) UnlockForChildLookup() { + d.mu.Unlock() +} + func (d *baseDirInode) ID() fuseops.InodeID { return d.id } diff --git a/internal/fs/inode/dir.go b/internal/fs/inode/dir.go index dba9b92bed..f4ca725dbd 100644 --- a/internal/fs/inode/dir.go +++ b/internal/fs/inode/dir.go @@ -132,6 +132,9 @@ type DirInode interface { // LocalFileEntries lists the local files present in the directory. // Local means that the file is not yet present on GCS. LocalFileEntries(localFileInodes map[Name]Inode) (localEntries []fuseutil.Dirent) + + LockForChildLookup() + UnlockForChildLookup() } // An inode that represents a directory from a GCS bucket. @@ -168,9 +171,9 @@ type dirInode struct { // Mutable state ///////////////////////// - // A mutex that must be held when calling certain methods. See documentation - // for each method. - mu locker.Locker + // A RW mutex that must be held when calling certain methods. See + // documentation for each method. + mu locker.RWLocker // GUARDED_BY(mu) lc lookupCount @@ -235,7 +238,7 @@ func NewDirInode( typed.lc.Init(id) // Set up invariant checking. - typed.mu = locker.New(name.GcsObjectName(), typed.checkInvariants) + typed.mu = locker.NewRW(name.GcsObjectName(), typed.checkInvariants) d = typed return @@ -386,6 +389,14 @@ func (d *dirInode) Unlock() { d.mu.Unlock() } +func (d *dirInode) LockForChildLookup() { + d.mu.RLock() +} + +func (d *dirInode) UnlockForChildLookup() { + d.mu.RUnlock() +} + func (d *dirInode) ID() fuseops.InodeID { return d.id } diff --git a/main_test.go b/main_test.go index 0d87ec221e..5af325b7f1 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}" + 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}" AssertEq(expected, actual) } @@ -188,7 +188,7 @@ 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}" + 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}" AssertEq(expected, actual) } diff --git a/mount.go b/mount.go index e769de31e8..1b024223ff 100644 --- a/mount.go +++ b/mount.go @@ -141,10 +141,11 @@ be interacting with the file system.`) // Mount the file system. logger.Infof("Mounting file system %q...", fsName) mountCfg := &fuse.MountConfig{ - FSName: fsName, - Subtype: "gcsfuse", - VolumeName: "gcsfuse", - Options: flags.MountOptions, + FSName: fsName, + Subtype: "gcsfuse", + VolumeName: "gcsfuse", + Options: flags.MountOptions, + EnableParallelDirOps: !(mountConfig.FileSystemConfig.DisableParallelDirops), } mountCfg.ErrorLogger = logger.NewLegacyLogger(logger.LevelError, "fuse: ")