[PR1] allow parallel lookups

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/googleapis/gax-go/v2 v2.12.4
 	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

go.sum

@@ -718,8 +718,8 @@ 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-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=

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
 }

internal/config/testdata/file_system_config/invalid_disable_parallel_dirops.yaml

@@ -0,0 +1,2 @@
+file-system:
+  disable-parallel-dirops: -1

internal/config/testdata/file_system_config/unset_disable_parallel_dirops.yaml

@@ -0,0 +1,2 @@
+write:
+  create-empty-file: true

internal/config/testdata/valid_config.yaml

@@ -25,3 +25,4 @@ grpc:
 enable-hns: true
 file-system:
   ignore-interrupts: true
+  disable-parallel-dirops: true

internal/config/yaml_parser_test.go

@@ -47,6 +47,7 @@ func validateDefaultConfig(t *testing.T, mountConfig *MountConfig) {
 	assert.False(t, mountConfig.AuthConfig.AnonymousAccess)
 	assert.False(t, bool(mountConfig.EnableHNS))
 	assert.False(t, mountConfig.FileSystemConfig.IgnoreInterrupts)
+	assert.False(t, mountConfig.FileSystemConfig.DisableParallelDirops)
 }
 
 func (t *YamlParserTest) TestReadConfigFile_EmptyFileName() {
@@ -126,6 +127,7 @@ func (t *YamlParserTest) TestReadConfigFile_ValidConfig() {
 
 	// file-system config
 	assert.True(t.T(), mountConfig.FileSystemConfig.IgnoreInterrupts)
+	assert.True(t.T(), mountConfig.FileSystemConfig.DisableParallelDirops)
 }
 
 func (t *YamlParserTest) TestReadConfigFile_InvalidLogConfig() {
@@ -247,3 +249,17 @@ func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_UnsetAnonymousAcces
 	assert.NotNil(t.T(), mountConfig)
 	assert.False(t.T(), mountConfig.AuthConfig.AnonymousAccess)
 }
+
+func (t *YamlParserTest) TestReadConfigFile_FileSystemConfig_InvalidDisableParallelDirops() {
+	_, err := ParseConfigFile("testdata/file_system_config/invalid_disable_parallel_dirops.yaml")
+
+	assert.ErrorContains(t.T(), err, "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")
+
+	assert.NoError(t.T(), err)
+	assert.NotNil(t.T(), mountConfig)
+	assert.False(t.T(), mountConfig.FileSystemConfig.DisableParallelDirops)
+}

internal/fs/fs.go

@@ -924,8 +924,15 @@ 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 {
+			// LockForChildLookup takes read-only or exclusive lock based on the
+			// inode when its child is looked up.
+			parent.LockForChildLookup()
+			defer parent.UnlockForChildLookup()
+		}
 		return parent.LookUpChild(ctx, childName)
 	}
 

internal/fs/inode/base_dir.go

@@ -92,6 +92,18 @@ func (d *baseDirInode) Unlock() {
 	d.mu.Unlock()
 }
 
+// LockForChildLookup takes exclusive lock on inode when the inode's child is
+// looked up. It is different from non-base dir inode because during lookup of
+// child in base directory inode, the buckets map is modified and hence should
+// be guarded by exclusive lock.
+func (d *baseDirInode) LockForChildLookup() {
+	d.mu.Lock()
+}
+
+func (d *baseDirInode) UnlockForChildLookup() {
+	d.mu.Unlock()
+}
+
 func (d *baseDirInode) ID() fuseops.InodeID {
 	return d.id
 }

internal/fs/inode/dir.go

@@ -132,6 +132,13 @@ 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 takes appropriate kind of lock when an inode's child is
+	// looked up.
+	LockForChildLookup()
+
+	// UnlockForChildLookup unlocks the lock taken with LockForChildLookup.
+	UnlockForChildLookup()
 }
 
 // An inode that represents a directory from a GCS bucket.
@@ -168,9 +175,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 +242,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 +393,19 @@ func (d *dirInode) Unlock() {
 	d.mu.Unlock()
 }
 
+// LockForChildLookup takes read-only lock on inode when the inode's child is
+// looked up. It is safe to take read-only lock to allow parallel lookups of
+// children because (a) during lookup, GCS is only read (list/stat), so as long
+// as GCS is not changed remotely, lookup will be consistent (b) all the other
+// directory level operations (read or write type) take exclusive locks.
+func (d *dirInode) LockForChildLookup() {
+	d.mu.RLock()
+}
+
+func (d *dirInode) UnlockForChildLookup() {
+	d.mu.RUnlock()
+}
+
 func (d *dirInode) ID() fuseops.InodeID {
 	return d.id
 }

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)
 }
 

mount.go

@@ -145,6 +145,15 @@ be interacting with the file system.`)
 		Subtype:    "gcsfuse",
 		VolumeName: "gcsfuse",
 		Options:    flags.MountOptions,
+		// Allows parallel LookUpInode & ReadDir calls from Kernel's FUSE driver.
+		// GCSFuse takes exclusive lock on directory inodes during ReadDir call,
+		// hence there is no effect of parallelization of incoming ReadDir calls
+		// from FUSE driver for user of GCSFuse. However, in case of LookUpInode
+		// calls, GCSFuse takes read only lock during LookUpInode call which helps
+		// users experience the performance gains. E.g. if a user workload tries to
+		// access two files under same directory parallely, then the lookups also
+		// happen parallely.
+		EnableParallelDirOps: !(mountConfig.FileSystemConfig.DisableParallelDirops),
 	}
 
 	mountCfg.ErrorLogger = logger.NewLegacyLogger(logger.LevelError, "fuse: ")