Revert "Add new file to type cache (#2303)"

This reverts commit 3cf1b91.

internal/fs/fs.go

@@ -1642,19 +1642,16 @@ func (fs *fileSystem) createFile(
 // Creates localFileInode with the given name under the parent inode.
 // LOCKS_EXCLUDED(fs.mu)
 // UNLOCK_FUNCTION(fs.mu)
-// LOCK_FUNCTION(child)
+// LOCK_FUNCTION(in)
 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()
 
@@ -1665,6 +1662,8 @@ func (fs *fileSystem) createLocalFile(
 		}
 	}()
 
+	fs.mu.Lock()
+
 	fullName := inode.NewFileName(parent.Name(), name)
 	child, ok := fs.localFileInodes[fullName]
 
@@ -1673,26 +1672,23 @@ 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.
-	core, err := parent.CreateLocalChildFileCore(name)
+	var result *inode.Core
+	result, err = parent.CreateLocalChildFile(name)
 	if err != nil {
 		return
 	}
-	child = fs.mintInode(core)
+
+	child = fs.mintInode(*result)
 	fs.localFileInodes[child.Name()] = child
 
 	// Empty file is created to be able to set attributes on the file.
 	fileInode := child.(*inode.FileInode)
-	if err := fileInode.CreateEmptyTempFile(); err != nil {
-		return nil, err
+	err = fileInode.CreateEmptyTempFile()
+	if err != nil {
+		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
+
+	return
 }
 
 // LOCKS_EXCLUDED(fs.mu)

internal/fs/inode/base_dir.go

@@ -199,12 +199,8 @@ func (d *baseDirInode) CreateChildFile(ctx context.Context, name string) (*Core,
 	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) CreateLocalChildFile(name string) (*Core, error) {
+	return nil, fuse.ENOSYS
 }
 
 func (d *baseDirInode) CloneToChildFile(ctx context.Context, name string, src *gcs.MinObject) (*Core, error) {

internal/fs/inode/dir.go

@@ -92,14 +92,9 @@ 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)
 
-	// 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)
+	// 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)
 
 	// Like CreateChildFile, except clone the supplied source object instead of
 	// creating an empty object.
@@ -811,25 +806,17 @@ func (d *dirInode) CreateChildFile(ctx context.Context, name string) (*Core, err
 	}, nil
 }
 
-func (d *dirInode) CreateLocalChildFileCore(name string) (Core, error) {
-	return Core{
+func (d *dirInode) CreateLocalChildFile(name string) (*Core, error) {
+	fullName := NewFileName(d.Name(), name)
+
+	return &Core{
 		Bucket:    d.Bucket(),
-		FullName:  NewFileName(d.Name(), name),
+		FullName:  fullName,
 		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,36 +1436,21 @@ func (t *DirTest) DeleteChildDir_ImplicitDirTrue() {
 	ExpectFalse(dirIn.IsUnlinked())
 }
 
-func (t *DirTest) LocalChildFileCore() {
-	core, err := t.in.CreateLocalChildFileCore("qux")
+func (t *DirTest) CreateLocalChildFile_ShouldnotCreateObjectInGCS() {
+	const name = "qux"
+
+	// Create the local file inode.
+	core, err := t.in.CreateLocalChildFile(name)
 
 	AssertEq(nil, err)
-	AssertEq(t.bucket.Name(), core.Bucket.Name())
-	AssertEq("foo/bar/qux", core.FullName.objectName)
-	AssertTrue(core.Local)
+	AssertEq(true, core.Local)
 	AssertEq(nil, core.MinObject)
-	result, err := t.in.LookUpChild(t.ctx, "qux")
+
+	// Object shouldn't get created in GCS.
+	result, err := t.in.LookUpChild(t.ctx, name)
 	AssertEq(nil, err)
 	AssertEq(nil, result)
-	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)
+	ExpectEq(metadata.UnknownType, t.getTypeFromCache(name))
 }
 
 func (t *DirTest) LocalFileEntriesEmpty() {

internal/fs/local_file_test.go

@@ -857,25 +857,3 @@ 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,54 +623,6 @@ 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)
 ////////////////////////////////////////////////////////////////////////