Add new file to type cache (#2303)

* Add new file to type-cache

internal/fs/fs.go

@@ -1642,16 +1642,19 @@ func (fs *fileSystem) createFile(
 // Creates localFileInode with the given name under the parent inode.
 // LOCKS_EXCLUDED(fs.mu)
 // UNLOCK_FUNCTION(fs.mu)
-// LOCK_FUNCTION(in)
+// LOCK_FUNCTION(child)
 func (fs *fileSystem) createLocalFile(
 	parentID fuseops.InodeID,
 	name string) (child inode.Inode, err error) {
 	// Find the parent.
 	fs.mu.Lock()
 	parent := fs.dirInodeOrDie(parentID)
-	fs.mu.Unlock()
 
 	defer func() {
+		if err != nil {
+			// fs.mu lock is already taken
+			delete(fs.localFileInodes, child.Name())
+		}
 		// We need to release the filesystem lock before acquiring the inode lock.
 		fs.mu.Unlock()
 
@@ -1662,8 +1665,6 @@ func (fs *fileSystem) createLocalFile(
 		}
 	}()
 
-	fs.mu.Lock()
-
 	fullName := inode.NewFileName(parent.Name(), name)
 	child, ok := fs.localFileInodes[fullName]
 
@@ -1672,23 +1673,26 @@ func (fs *fileSystem) createLocalFile(
 	}
 
 	// Create a new inode when a file is created first time, or when a local file is unlinked and then recreated with the same name.
-	var result *inode.Core
-	result, err = parent.CreateLocalChildFile(name)
+	core, err := parent.CreateLocalChildFileCore(name)
 	if err != nil {
 		return
 	}
-
-	child = fs.mintInode(*result)
+	child = fs.mintInode(core)
 	fs.localFileInodes[child.Name()] = child
 
 	// Empty file is created to be able to set attributes on the file.
 	fileInode := child.(*inode.FileInode)
-	err = fileInode.CreateEmptyTempFile()
-	if err != nil {
-		return
+	if err := fileInode.CreateEmptyTempFile(); err != nil {
+		return nil, err
 	}
-
-	return
+	fs.mu.Unlock()
+	parent.Lock()
+	defer parent.Unlock()
+	parent.InsertFileIntoTypeCache(name)
+	// Even though there is no action here that requires locking, adding locking
+	// so that the defer call that unlocks the mutex doesn't fail.
+	fs.mu.Lock()
+	return child, nil
 }
 
 // LOCKS_EXCLUDED(fs.mu)

internal/fs/inode/base_dir.go

@@ -199,8 +199,12 @@ func (d *baseDirInode) CreateChildFile(ctx context.Context, name string) (*Core,
 	return nil, fuse.ENOSYS
 }
 
-func (d *baseDirInode) CreateLocalChildFile(name string) (*Core, error) {
-	return nil, fuse.ENOSYS
+func (d *baseDirInode) InsertFileIntoTypeCache(_ string) {}
+
+func (d *baseDirInode) EraseFromTypeCache(_ string) {}
+
+func (d *baseDirInode) CreateLocalChildFileCore(_ string) (Core, error) {
+	return Core{}, fuse.ENOSYS
 }
 
 func (d *baseDirInode) CloneToChildFile(ctx context.Context, name string, src *gcs.MinObject) (*Core, error) {

internal/fs/inode/dir.go

@@ -92,9 +92,14 @@ type DirInode interface {
 	// Return the full name of the child and the GCS object it backs up.
 	CreateChildFile(ctx context.Context, name string) (*Core, error)
 
-	// Create an empty local child file with the supplied (relative) name. Local
-	// file means the object is not yet created in GCS.
-	CreateLocalChildFile(name string) (*Core, error)
+	// CreateLocalChildFileCore returns an empty local child file core.
+	CreateLocalChildFileCore(name string) (Core, error)
+
+	// InsertFileIntoTypeCache adds the given name to type-cache
+	InsertFileIntoTypeCache(name string)
+
+	// EraseFromTypeCache removes the given name from type-cache
+	EraseFromTypeCache(name string)
 
 	// Like CreateChildFile, except clone the supplied source object instead of
 	// creating an empty object.
@@ -806,17 +811,25 @@ func (d *dirInode) CreateChildFile(ctx context.Context, name string) (*Core, err
 	}, nil
 }
 
-func (d *dirInode) CreateLocalChildFile(name string) (*Core, error) {
-	fullName := NewFileName(d.Name(), name)
-
-	return &Core{
+func (d *dirInode) CreateLocalChildFileCore(name string) (Core, error) {
+	return Core{
 		Bucket:    d.Bucket(),
-		FullName:  fullName,
+		FullName:  NewFileName(d.Name(), name),
 		MinObject: nil,
 		Local:     true,
 	}, nil
 }
 
+// LOCKS_REQUIRED(d)
+func (d *dirInode) InsertFileIntoTypeCache(name string) {
+	d.cache.Insert(d.cacheClock.Now(), name, metadata.RegularFileType)
+}
+
+// LOCKS_REQUIRED(d)
+func (d *dirInode) EraseFromTypeCache(name string) {
+	d.cache.Erase(name)
+}
+
 // LOCKS_REQUIRED(d)
 func (d *dirInode) CloneToChildFile(ctx context.Context, name string, src *gcs.MinObject) (*Core, error) {
 	// Erase any existing type information for this name.

internal/fs/inode/dir_test.go

@@ -1436,21 +1436,36 @@ func (t *DirTest) DeleteChildDir_ImplicitDirTrue() {
 	ExpectFalse(dirIn.IsUnlinked())
 }
 
-func (t *DirTest) CreateLocalChildFile_ShouldnotCreateObjectInGCS() {
-	const name = "qux"
-
-	// Create the local file inode.
-	core, err := t.in.CreateLocalChildFile(name)
+func (t *DirTest) LocalChildFileCore() {
+	core, err := t.in.CreateLocalChildFileCore("qux")
 
 	AssertEq(nil, err)
-	AssertEq(true, core.Local)
+	AssertEq(t.bucket.Name(), core.Bucket.Name())
+	AssertEq("foo/bar/qux", core.FullName.objectName)
+	AssertTrue(core.Local)
 	AssertEq(nil, core.MinObject)
-
-	// Object shouldn't get created in GCS.
-	result, err := t.in.LookUpChild(t.ctx, name)
+	result, err := t.in.LookUpChild(t.ctx, "qux")
 	AssertEq(nil, err)
 	AssertEq(nil, result)
-	ExpectEq(metadata.UnknownType, t.getTypeFromCache(name))
+	ExpectEq(metadata.UnknownType, t.getTypeFromCache("qux"))
+}
+
+func (t *DirTest) InsertIntoTypeCache() {
+	t.in.InsertFileIntoTypeCache("abc")
+
+	d := t.in.(*dirInode)
+	tp := t.tc.Get(d.cacheClock.Now(), "abc")
+	AssertEq(2, tp)
+}
+
+func (t *DirTest) EraseFromTypeCache() {
+	t.in.InsertFileIntoTypeCache("abc")
+
+	t.in.EraseFromTypeCache("abc")
+
+	d := t.in.(*dirInode)
+	tp := d.cache.Get(d.cacheClock.Now(), "abc")
+	AssertEq(0, tp)
 }
 
 func (t *DirTest) LocalFileEntriesEmpty() {

internal/fs/local_file_test.go

@@ -857,3 +857,25 @@ func (t *LocalFileTest) TestStatLocalFileAfterRecreatingItWithSameName() {
 	ExpectEq("test.txt", f.Name())
 	ExpectFalse(f.IsDir())
 }
+
+func (t *LocalFileTest) TestStatFailsOnNewFileAfterDeletion() {
+	t.serverCfg.NewConfig = &cfg.Config{
+		ImplicitDirs: true,
+		MetadataCache: cfg.MetadataCacheConfig{
+			TtlSecs:            -1,
+			TypeCacheMaxSizeMb: -1,
+			StatCacheMaxSizeMb: -1,
+		},
+		Logging: cfg.DefaultLoggingConfig(),
+	}
+	filePath := path.Join(mntDir, "test.txt")
+	AssertEq(nil, err)
+	f1, err := os.Create(filePath)
+	AssertEq(nil, err)
+	defer AssertEq(nil, f1.Close())
+	AssertEq(nil, os.Remove(filePath))
+
+	_, err = os.Stat(filePath)
+
+	AssertNe(nil, err)
+}

tools/integration_tests/concurrent_operations/concurrent_listing_test.go

@@ -623,6 +623,54 @@ func (s *concurrentListingTest) Test_ListWithMoveDir(t *testing.T) {
 	}
 }
 
+// Test_StatWithNewFileWrite tests for potential deadlocks or race conditions when
+// statting and creating a new file happen concurrently.
+func (s *concurrentListingTest) Test_StatWithNewFileWrite(t *testing.T) {
+	t.Parallel()
+	testCaseDir := "Test_StatWithNewFileWrite"
+	createDirectoryStructureForTestCase(t, testCaseDir)
+	targetDir := path.Join(testDirPath, testCaseDir, "explicitDir")
+	var wg sync.WaitGroup
+	wg.Add(2)
+	timeout := 400 * time.Second // Adjust timeout as needed
+
+	// Goroutine 1: Repeatedly calls Stat
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iterationsForLightOperations; i++ {
+			_, err := os.Stat(targetDir)
+
+			assert.NoError(t, err)
+		}
+	}()
+
+	// Goroutine 2: Repeatedly create a file.
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iterationsForLightOperations; i++ {
+			// Create file
+			filePath := path.Join(targetDir, fmt.Sprintf("tmp_file_%d.txt", i))
+			err := os.WriteFile(filePath, []byte("Hello, world!"), setup.FilePermission_0600)
+
+			assert.NoError(t, err)
+		}
+	}()
+
+	// Wait for goroutines or timeout
+	done := make(chan bool)
+	go func() {
+		wg.Wait()
+		done <- true
+	}()
+
+	select {
+	case <-done:
+		// Success: Both operations finished before timeout
+	case <-time.After(timeout):
+		assert.FailNow(t, "Possible deadlock or race condition detected")
+	}
+}
+
 ////////////////////////////////////////////////////////////////////////
 // Test Function (Runs once before all tests)
 ////////////////////////////////////////////////////////////////////////